Skip to content

Commit c917635

Browse files
cognifloydcognifloyd
authored
Merge pull request #5332 from st2sandbox/pylint-plugins-use-ast
Make our custom pylint plugin use the AST
2 parents 5f85c76 + d67c7a6 commit c917635

File tree

3 files changed

+546
-47
lines changed

3 files changed

+546
-47
lines changed

‎Makefile

+4
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,10 @@ schemasgen: requirements .schemasgen
336336
@echo
337337
@echo "================== pylint ===================="
338338
@echo
339+
@echo "==========================================================="; \
340+
echo "Test our custom pylint plugins before we use them"; \
341+
echo "==========================================================="; \
342+
. $(VIRTUALENV_DIR)/bin/activate ; pytest pylint_plugins || exit 1
339343
# Lint st2 components
340344
@for component in $(COMPONENTS); do\
341345
echo "==========================================================="; \

‎pylint_plugins/api_models.py

+246-47
Original file line numberDiff line numberDiff line change
@@ -19,72 +19,271 @@
1919
2020
Those classes dyamically assign attributes defined in the schema on the class inside the
2121
constructor.
22+
23+
Pylint uses static analysis on the python files it lints. This means that it does not
24+
import any of the code using standard python libraries. Instead, it parses them into an
25+
AST using the astroid library. Thus, it is safe to run Pylint on code that would
26+
have import time side-effects without triggering those effects. When parsing a single file,
27+
Pylint can parse the direct dependencies of that file without following the entire
28+
import chain, which might include significant transient dependencies in 3rd party libraries.
29+
30+
Since pylint is using an AST instead of importing the code, it cannot know about the dynamic
31+
attributes that get added to our API model classes. Plus, the schema itself is often
32+
constructed by including copies of common sub schemas. So, the attributes are dynamic AND
33+
the schemas that define those attributes are also dynamic.
34+
35+
So, in this plugin we have to do a bit of work to:
36+
37+
1) extract the "schema =" assignment,
38+
2) parse the assigned value to find any other variables used in the schema object,
39+
3) extract the assignments for those other variables, and
40+
4) construct a final dictionary AST node that includes all the attributes that
41+
Pylint needs to know about on our model classes.
42+
43+
At this point, we have the schema, so then we:
44+
45+
5) iterate over the schema properties,
46+
6) parse each property's value to find any other referenced variables,
47+
7) extract the assignments for those referenced variables,
48+
8) inspect the types of those properties (str, int, list, etc), and
49+
9) add new attribute AST nodes (of the appropriate type) to the class AST node.
50+
51+
Now, we return because Pylint can finally understand our API model objects without
52+
importing them.
2253
"""
2354

2455
import astroid
25-
import six
2656

2757
from astroid import MANAGER
2858
from astroid import nodes
2959
from astroid import scoped_nodes
3060

3161
# A list of class names for which we want to skip the checks
32-
CLASS_NAME_BLACKLIST = ["ExecutionSpecificationAPI"]
62+
CLASS_NAME_SKIPLIST = ["ExecutionSpecificationAPI"]
3363

3464

3565
def register(linter):
3666
pass
3767

3868

39-
def transform(cls):
40-
if cls.name in CLASS_NAME_BLACKLIST:
69+
def infer_copy_deepcopy(call_node):
70+
"""
71+
Look for a call_node (ie a function call) like this:
72+
schema = copy.deepcopy(...)
73+
^^^^^^^^^^^^^
74+
Ignore any function calls that are not copy.deepcopy().
75+
"""
76+
if not (
77+
isinstance(call_node, nodes.Call)
78+
and call_node.func.as_string() == "copy.deepcopy"
79+
):
4180
return
81+
return next(call_node.args[0].infer())
82+
83+
84+
def predicate(cls: nodes.ClassDef) -> bool:
85+
"""
86+
Astroid (used by pylint) calls this to see if our transform function needs to run.
87+
"""
88+
if cls.name in CLASS_NAME_SKIPLIST:
89+
# class looks like an API model class, but it isn't.
90+
return False
91+
92+
if not cls.name.endswith("API") and "schema" not in cls.locals:
93+
# class does not look like an API model class.
94+
return False
4295

43-
if cls.name.endswith("API") or "schema" in cls.locals:
44-
# This is a class which defines attributes in "schema" variable using json schema.
45-
# Those attributes are then assigned during run time inside the constructor
46-
fqdn = cls.qname()
47-
module_name, class_name = fqdn.rsplit(".", 1)
96+
return True
4897

49-
module = __import__(module_name, fromlist=[class_name])
50-
actual_cls = getattr(module, class_name)
5198

52-
schema = actual_cls.schema
99+
def transform(cls: nodes.ClassDef):
100+
"""
101+
Astroid (used by pylint) calls this function on each class definition it discovers.
102+
cls is an Astroid AST representation of that class.
53103
54-
if not isinstance(schema, dict):
55-
# Not a class we are interested in
104+
Our purpose here is to extract the schema dict from API model classes
105+
so that we can inform pylint about all of the attributes on those models.
106+
We do this by injecting attributes on the class for each property in the schema.
107+
"""
108+
109+
# This is a class which defines attributes in "schema" variable using json schema.
110+
# Those attributes are then assigned during run time inside the constructor
111+
112+
# Get the value node for the "schema =" assignment
113+
schema_dict_node = next(cls.igetattr("schema"))
114+
115+
extra_schema_properties = {}
116+
117+
# If the "schema =" assignment's value node is not a simple type (like a dictionary),
118+
# then pylint cannot infer exactly what it does. Most of the time, this is actually
119+
# a function call to copy the schema from another class. So, let's find the dictionary.
120+
if schema_dict_node is astroid.Uninferable:
121+
# the assignment probably looks like this:
122+
# schema = copy.deepcopy(ActionAPI.schema)
123+
124+
# so far we only have the value, but we need the actual assignment
125+
assigns = [n for n in cls.get_children() if isinstance(n, nodes.Assign)]
126+
schema_assign_name_node = cls.local_attr("schema")[0]
127+
schema_assign_node = next(
128+
assign for assign in assigns if assign.targets[0] == schema_assign_name_node
129+
)
130+
assigns.remove(schema_assign_node)
131+
132+
# We only care about "schema = copy.deepcopy(...)"
133+
schema_dict_node = infer_copy_deepcopy(schema_assign_node.value)
134+
if not schema_dict_node:
135+
# This is not an API model class, as it doesn't have
136+
# something we can resolve to a dictionary.
56137
return
57138

58-
properties = schema.get("properties", {})
59-
for property_name, property_data in six.iteritems(properties):
60-
property_name = property_name.replace(
61-
"-", "_"
62-
) # Note: We do the same in Python code
63-
property_type = property_data.get("type", None)
64-
65-
if isinstance(property_type, (list, tuple)):
66-
# Hack for attributes with multiple types (e.g. string, null)
67-
property_type = property_type[0]
68-
69-
if property_type == "object":
70-
node = nodes.Dict()
71-
elif property_type == "array":
72-
node = nodes.List()
73-
elif property_type == "integer":
74-
node = scoped_nodes.builtin_lookup("int")[1][0]
75-
elif property_type == "number":
76-
node = scoped_nodes.builtin_lookup("float")[1][0]
77-
elif property_type == "string":
78-
node = scoped_nodes.builtin_lookup("str")[1][0]
79-
elif property_type == "boolean":
80-
node = scoped_nodes.builtin_lookup("bool")[1][0]
81-
elif property_type == "null":
82-
node = scoped_nodes.builtin_lookup("None")[1][0]
83-
else:
84-
# Unknown type
85-
node = astroid.ClassDef(property_name, None)
86-
87-
cls.locals[property_name] = [node]
88-
89-
90-
MANAGER.register_transform(astroid.ClassDef, transform)
139+
# OK, now we need to look for any properties that dynamically modify
140+
# the dictionary that was just copied from somewhere else.
141+
# See the note below for why we only care about "properties" here.
142+
for assign_node in assigns:
143+
# we're looking for assignments like this:
144+
# schema["properties"]["ttl"] = {...}
145+
target = assign_node.targets[0]
146+
try:
147+
if (
148+
isinstance(target, nodes.Subscript)
149+
and target.value.value.name == "schema"
150+
and target.value.slice.value.value == "properties"
151+
):
152+
property_name_node = target.slice.value
153+
else:
154+
# not schema["properties"]
155+
continue
156+
except AttributeError:
157+
continue
158+
159+
# schema["properties"]["execution"] = copy.deepcopy(ActionExecutionAPI.schema)
160+
inferred_value = infer_copy_deepcopy(assign_node.value)
161+
162+
extra_schema_properties[property_name_node] = (
163+
inferred_value if inferred_value else assign_node.value
164+
)
165+
166+
if not isinstance(schema_dict_node, nodes.Dict):
167+
# Not a class we are interested in (like BaseAPI)
168+
return
169+
170+
# We only care about "properties" in the schema because that's the only part of the schema
171+
# that gets translated into dynamic attributes on the model API class.
172+
properties_dict_node = None
173+
for key_node, value_node in schema_dict_node.items:
174+
if key_node.value == "properties":
175+
properties_dict_node = value_node
176+
break
177+
178+
if not properties_dict_node and not extra_schema_properties:
179+
# Not a class we can do anything with
180+
return
181+
182+
# Hooray! We have the schema properties dict now, so we can start processing
183+
# each property and add an attribute for each one to the API model class node.
184+
for property_name_node, property_data_node in properties_dict_node.items + list(
185+
extra_schema_properties.items()
186+
):
187+
property_name = property_name_node.value.replace(
188+
"-", "_"
189+
) # Note: We do the same in Python code
190+
191+
# Despite the processing above to extract the schema properties dictionary
192+
# each property in the dictionary might also reference other variables,
193+
# so we still need to resolve these to figure out each property's type.
194+
195+
# an indirect reference to copy.deepcopy() as in:
196+
# REQUIRED_ATTR_SCHEMAS = {"action": copy.deepcopy(ActionAPI.schema)}
197+
# schema = {"properties": {"action": REQUIRED_ATTR_SCHEMAS["action"]}}
198+
if isinstance(property_data_node, nodes.Subscript):
199+
var_name = property_data_node.value.name
200+
subscript = property_data_node.slice.value.value
201+
202+
# lookup var by name (assume its at module level)
203+
var_node = next(cls.root().igetattr(var_name))
204+
205+
# assume it is a dict at this point
206+
data_node = None
207+
for key_node, value_node in var_node.items:
208+
if key_node.value == subscript:
209+
# infer will resolve a Dict
210+
data_node = next(value_node.infer())
211+
if data_node is astroid.Uninferable:
212+
data_node = infer_copy_deepcopy(value_node)
213+
break
214+
if data_node:
215+
property_data_node = data_node
216+
217+
if not isinstance(property_data_node, nodes.Dict):
218+
# if infer_copy_deepcopy already ran, we may need to resolve the dict
219+
data_node = next(property_data_node.infer())
220+
if data_node is not astroid.Uninferable:
221+
property_data_node = data_node
222+
223+
property_type_node = None
224+
if isinstance(property_data_node, nodes.Dict):
225+
# We have a property schema, but we only care about the property's type.
226+
for property_key_node, property_value_node in property_data_node.items:
227+
if property_key_node.value == "type":
228+
property_type_node = next(property_value_node.infer())
229+
break
230+
231+
if property_type_node is None and isinstance(
232+
property_data_node, nodes.Attribute
233+
):
234+
# reference schema from another file like this:
235+
# from ... import TriggerAPI
236+
# schema = {"properties": {"trigger": TriggerAPI.schema}}
237+
# We only pull a schema from another file when it is an "object" (a dict).
238+
# So, we do not need to do any difficult cross-file processing.
239+
property_type = "object"
240+
elif property_type_node is None:
241+
property_type = None
242+
elif isinstance(property_type_node, nodes.Const):
243+
property_type = property_type_node.value
244+
elif isinstance(property_type_node, (nodes.List, nodes.Tuple)):
245+
# Hack for attributes with multiple types (e.g. string, null)
246+
property_type = property_type_node.elts[
247+
0
248+
].value # elts has "elements" in the list/tuple
249+
else:
250+
# We should only hit this if someone has used a different approach
251+
# for dynamically constructing the property's schema.
252+
# Expose the AST at this point to facilitate handling that approach.
253+
raise Exception(property_type_node.repr_tree())
254+
255+
# Hooray! We've got a property's name at this point.
256+
# And we have the property's type, if that type was defined in the schema.
257+
# Now, we can construct the AST node that we'll add to the API model class.
258+
259+
if property_type == "object":
260+
node = nodes.Dict()
261+
elif property_type == "array":
262+
node = nodes.List()
263+
elif property_type == "integer":
264+
node = scoped_nodes.builtin_lookup("int")[1][0]
265+
elif property_type == "number":
266+
node = scoped_nodes.builtin_lookup("float")[1][0]
267+
elif property_type == "string":
268+
node = scoped_nodes.builtin_lookup("str")[1][0]
269+
elif property_type == "boolean":
270+
node = scoped_nodes.builtin_lookup("bool")[1][0]
271+
elif property_type == "null":
272+
node = scoped_nodes.builtin_lookup("None")[1][0]
273+
else:
274+
# Unknown type
275+
node = astroid.ClassDef(property_name, None)
276+
277+
# Create a "property = node" assign node
278+
assign_node = nodes.Assign(parent=cls)
279+
assign_name_node = nodes.AssignName(property_name, parent=assign_node)
280+
assign_node.postinit(targets=[assign_name_node], value=node)
281+
282+
# Finally, add the property node as an attribute on the class.
283+
cls.locals[property_name] = [assign_name_node]
284+
285+
# Now, pylint should be aware of all of the properties that get dynamically
286+
# added as attributes on the API model class.
287+
288+
289+
MANAGER.register_transform(astroid.ClassDef, transform, predicate)

0 commit comments

Comments
 (0)