mirror of
https://github.com/github/codeql.git
synced 2025-12-21 19:26:31 +01:00
Python: Fix missing module resolution
This was due to bad manual magic: restricting the attribute name makes sense when we're talking about submodules of a package, but it doesn't when we're talking about reexported modules. Also (hopefully) fixes the tests so that the Python 3-specific bits are ignored under Python 2.
This commit is contained in:
@@ -235,9 +235,12 @@ module ImportResolution {
|
|||||||
result = ar
|
result = ar
|
||||||
|
|
|
|
||||||
isPreferredModuleForName(m.getFile(), p.getPackageName() + "." + attr_name + ["", ".__init__"])
|
isPreferredModuleForName(m.getFile(), p.getPackageName() + "." + attr_name + ["", ".__init__"])
|
||||||
|
)
|
||||||
or
|
or
|
||||||
// This is also true for attributes that come from reexports.
|
// This is also true for attributes that come from reexports.
|
||||||
module_reexport(p, attr_name, m)
|
exists(Module reexporter, string attr_name |
|
||||||
|
result.(DataFlow::AttrRead).accesses(getModuleReference(reexporter), attr_name) and
|
||||||
|
module_reexport(reexporter, attr_name, m)
|
||||||
)
|
)
|
||||||
or
|
or
|
||||||
// Submodules that are implicitly defined with relative imports of the form `from .foo import ...`.
|
// Submodules that are implicitly defined with relative imports of the form `from .foo import ...`.
|
||||||
|
|||||||
@@ -74,7 +74,7 @@ class ResolutionTest extends InlineExpectationsTest {
|
|||||||
(
|
(
|
||||||
exists(DataFlow::PathNode source, DataFlow::PathNode sink, ImportConfiguration config |
|
exists(DataFlow::PathNode source, DataFlow::PathNode sink, ImportConfiguration config |
|
||||||
config.hasFlowPath(source, sink) and
|
config.hasFlowPath(source, sink) and
|
||||||
correct_version(sink.getNode()) and
|
not sink.getNode() instanceof VersionGuardedNode and
|
||||||
tag = "prints" and
|
tag = "prints" and
|
||||||
location = sink.getNode().getLocation() and
|
location = sink.getNode().getLocation() and
|
||||||
value = source.getNode().(SourceString).getContents() and
|
value = source.getNode().(SourceString).getContents() and
|
||||||
@@ -82,7 +82,7 @@ class ResolutionTest extends InlineExpectationsTest {
|
|||||||
)
|
)
|
||||||
or
|
or
|
||||||
exists(ModuleRef ref |
|
exists(ModuleRef ref |
|
||||||
correct_version(ref) and
|
not ref instanceof VersionGuardedNode and
|
||||||
ref instanceof CheckArgument and
|
ref instanceof CheckArgument and
|
||||||
tag = "prints" and
|
tag = "prints" and
|
||||||
location = ref.getLocation() and
|
location = ref.getLocation() and
|
||||||
@@ -93,7 +93,30 @@ class ResolutionTest extends InlineExpectationsTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private predicate correct_version(DataFlow::Node n) {
|
class ResolutionTest3 extends InlineExpectationsTest {
|
||||||
not n instanceof VersionGuardedNode or
|
ResolutionTest3() { this = "ResolutionTest3" }
|
||||||
n.(VersionGuardedNode).getVersion() = major_version()
|
|
||||||
|
override string getARelevantTag() { result = "prints3" }
|
||||||
|
|
||||||
|
override predicate hasActualResult(Location location, string element, string tag, string value) {
|
||||||
|
(
|
||||||
|
exists(DataFlow::PathNode source, DataFlow::PathNode sink, ImportConfiguration config |
|
||||||
|
config.hasFlowPath(source, sink) and
|
||||||
|
sink.getNode().(VersionGuardedNode).getVersion() = 3 and
|
||||||
|
tag = "prints3" and
|
||||||
|
location = sink.getNode().getLocation() and
|
||||||
|
value = source.getNode().(SourceString).getContents() and
|
||||||
|
element = sink.getNode().toString()
|
||||||
|
)
|
||||||
|
or
|
||||||
|
exists(ModuleRef ref |
|
||||||
|
ref.(VersionGuardedNode).getVersion() = 3 and
|
||||||
|
ref instanceof CheckArgument and
|
||||||
|
tag = "prints3" and
|
||||||
|
location = ref.getLocation() and
|
||||||
|
value = "\"<module " + ref.getName() + ">\"" and
|
||||||
|
element = ref.toString()
|
||||||
|
)
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -35,7 +35,8 @@ import foo as foo_alias #$ imports=foo as=foo_alias
|
|||||||
check("foo_alias.foo_attr", foo_alias.foo_attr, "foo_attr", globals()) #$ prints=foo_attr
|
check("foo_alias.foo_attr", foo_alias.foo_attr, "foo_attr", globals()) #$ prints=foo_attr
|
||||||
|
|
||||||
# A reference to a reexported module
|
# A reference to a reexported module
|
||||||
check("foo.bar_reexported.bar_attr", foo.bar_reexported.bar_attr, "bar_attr", globals()) #$ MISSING: prints=bar_attr
|
check("foo.bar_reexported", foo.bar_reexported, "<module bar>", globals()) #$ prints="<module bar>"
|
||||||
|
check("foo.bar_reexported.bar_attr", foo.bar_reexported.bar_attr, "bar_attr", globals()) #$ prints=bar_attr
|
||||||
|
|
||||||
# A simple "import from" statement.
|
# A simple "import from" statement.
|
||||||
from bar import bar_attr
|
from bar import bar_attr
|
||||||
@@ -71,7 +72,7 @@ check("package.package_attr", package.package_attr, "package_attr", globals()) #
|
|||||||
if sys.version_info[0] == 3:
|
if sys.version_info[0] == 3:
|
||||||
# Importing from a namespace module.
|
# Importing from a namespace module.
|
||||||
from namespace_package.namespace_module import namespace_module_attr
|
from namespace_package.namespace_module import namespace_module_attr
|
||||||
check("namespace_module_attr", namespace_module_attr, "namespace_module_attr", globals()) #$ prints=namespace_module_attr
|
check("namespace_module_attr", namespace_module_attr, "namespace_module_attr", globals()) #$ prints3=namespace_module_attr
|
||||||
|
|
||||||
|
|
||||||
from attr_clash import clashing_attr, non_clashing_submodule #$ imports=attr_clash.clashing_attr as=clashing_attr imports=attr_clash.non_clashing_submodule as=non_clashing_submodule
|
from attr_clash import clashing_attr, non_clashing_submodule #$ imports=attr_clash.clashing_attr as=clashing_attr imports=attr_clash.non_clashing_submodule as=non_clashing_submodule
|
||||||
|
|||||||
Reference in New Issue
Block a user