Skip to content

Commit 9dfb68f

Browse files
aptenodytes-forsterialexgartner-bc
aptenodytes-forsteri
authored and
alexgartner-bc
committed
Support "from" imports in the python gazelle plugin.
Fixes bazel-contrib#709. Test case for pip imports using "from foo import bar". Test cases for imports of the form "from foo import bar". Remove unnecessary flag (replaced with continue's). Make sure from imports work with std modules. - Add test case with `from __future__ import print_function`. Guard python_interpreter_target workspace name on None type check. (bazel-contrib#755) Fix for requirements_lock with PEP440 direct references (bazel-contrib#756) Fix indentation error. - Make sure that `from foo import bar, baz` works. - Add test case for this.
1 parent 0e75722 commit 9dfb68f

File tree

47 files changed

+1996
-94
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+1996
-94
lines changed

‎gazelle/parse.py

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,18 @@ def parse_import_statements(content, filepath):
2121
"name": subnode.name,
2222
"lineno": node.lineno,
2323
"filepath": filepath,
24+
"from": ""
2425
}
2526
modules.append(module)
2627
elif isinstance(node, ast.ImportFrom) and node.level == 0:
27-
module = {
28-
"name": node.module,
29-
"lineno": node.lineno,
30-
"filepath": filepath,
31-
}
32-
modules.append(module)
28+
for subnode in node.names:
29+
module = {
30+
"name": f"{node.module}.{subnode.name}",
31+
"lineno": node.lineno,
32+
"filepath": filepath,
33+
"from": node.module
34+
}
35+
modules.append(module)
3336
return modules
3437

3538

@@ -47,9 +50,10 @@ def parse(repo_root, rel_package_path, filename):
4750
abs_filepath = os.path.join(repo_root, rel_filepath)
4851
with open(abs_filepath, "r") as file:
4952
content = file.read()
50-
# From simple benchmarks, 2 workers gave the best performance here.
53+
# From simple benchmarks, 2 workers gave the best performance here.
5154
with concurrent.futures.ThreadPoolExecutor(max_workers=2) as executor:
52-
modules_future = executor.submit(parse_import_statements, content, rel_filepath)
55+
modules_future = executor.submit(parse_import_statements, content,
56+
rel_filepath)
5357
comments_future = executor.submit(parse_comments, content)
5458
modules = modules_future.result()
5559
comments = comments_future.result()
@@ -69,11 +73,12 @@ def main(stdin, stdout):
6973
filenames = parse_request["filenames"]
7074
outputs = list()
7175
if len(filenames) == 1:
72-
outputs.append(parse(repo_root, rel_package_path, filenames[0]))
76+
outputs.append(parse(repo_root, rel_package_path,
77+
filenames[0]))
7378
else:
7479
futures = [
75-
executor.submit(parse, repo_root, rel_package_path, filename)
76-
for filename in filenames
80+
executor.submit(parse, repo_root, rel_package_path,
81+
filename) for filename in filenames
7782
if filename != ""
7883
]
7984
for future in concurrent.futures.as_completed(futures):

‎gazelle/parser.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,13 +133,13 @@ func (p *python3Parser) parse(pyFilenames *treeset.Set) (*treeset.Set, error) {
133133
for _, m := range res.Modules {
134134
// Check for ignored dependencies set via an annotation to the Python
135135
// module.
136-
if annotations.ignores(m.Name) {
136+
if annotations.ignores(m.Name) || annotations.ignores(m.From) {
137137
continue
138138
}
139139

140140
// Check for ignored dependencies set via a Gazelle directive in a BUILD
141141
// file.
142-
if p.ignoresDependency(m.Name) {
142+
if p.ignoresDependency(m.Name) || p.ignoresDependency(m.From) {
143143
continue
144144
}
145145

@@ -170,6 +170,9 @@ type module struct {
170170
LineNumber uint32 `json:"lineno"`
171171
// The path to the module file relative to the Bazel workspace root.
172172
Filepath string `json:"filepath"`
173+
// If this was a from import, e.g. from foo import bar, From indicates the module
174+
// from which it is imported.
175+
From string `json:"from"`
173176
}
174177

175178
// moduleComparator compares modules by name.

‎gazelle/resolve.go

Lines changed: 103 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -140,99 +140,122 @@ func (py *Resolver) Resolve(
140140
it := modules.Iterator()
141141
explainDependency := os.Getenv("EXPLAIN_DEPENDENCY")
142142
hasFatalError := false
143-
MODULE_LOOP:
143+
MODULES_LOOP:
144144
for it.Next() {
145145
mod := it.Value().(module)
146-
imp := resolve.ImportSpec{Lang: languageName, Imp: mod.Name}
147-
if override, ok := resolve.FindRuleWithOverride(c, imp, languageName); ok {
148-
if override.Repo == "" {
149-
override.Repo = from.Repo
150-
}
151-
if !override.Equal(from) {
152-
if override.Repo == from.Repo {
153-
override.Repo = ""
154-
}
155-
dep := override.String()
156-
deps.Add(dep)
157-
if explainDependency == dep {
158-
log.Printf("Explaining dependency (%s): "+
159-
"in the target %q, the file %q imports %q at line %d, "+
160-
"which resolves using the \"gazelle:resolve\" directive.\n",
161-
explainDependency, from.String(), mod.Filepath, mod.Name, mod.LineNumber)
146+
moduleParts := strings.Split(mod.Name, ".")
147+
possibleModules := []string{mod.Name}
148+
for len(moduleParts) > 1 {
149+
// Iterate back through the possible imports until
150+
// a match is found.
151+
// For example, "from foo.bar import baz" where bar is a variable, we should try
152+
// `foo.bar.baz` first, then `foo.bar`, then `foo`. In the first case, the import could be file `baz.py`
153+
// in the directory `foo/bar`.
154+
// Or, the import could be variable `bar` in file `foo/bar.py`.
155+
// The import could also be from a standard module, e.g. `six.moves`, where
156+
// the dependency is actually `six`.
157+
moduleParts = moduleParts[:len(moduleParts)-1]
158+
possibleModules = append(possibleModules, strings.Join(moduleParts, "."))
159+
}
160+
errs := []error{}
161+
POSSIBLE_MODULE_LOOP:
162+
for _, moduleName := range possibleModules {
163+
imp := resolve.ImportSpec{Lang: languageName, Imp: moduleName}
164+
if override, ok := resolve.FindRuleWithOverride(c, imp, languageName); ok {
165+
if override.Repo == "" {
166+
override.Repo = from.Repo
162167
}
163-
}
164-
} else {
165-
if dep, ok := cfg.FindThirdPartyDependency(mod.Name); ok {
166-
deps.Add(dep)
167-
if explainDependency == dep {
168-
log.Printf("Explaining dependency (%s): "+
169-
"in the target %q, the file %q imports %q at line %d, "+
170-
"which resolves from the third-party module %q from the wheel %q.\n",
171-
explainDependency, from.String(), mod.Filepath, mod.Name, mod.LineNumber, mod.Name, dep)
168+
if !override.Equal(from) {
169+
if override.Repo == from.Repo {
170+
override.Repo = ""
171+
}
172+
dep := override.String()
173+
deps.Add(dep)
174+
if explainDependency == dep {
175+
log.Printf("Explaining dependency (%s): "+
176+
"in the target %q, the file %q imports %q at line %d, "+
177+
"which resolves using the \"gazelle:resolve\" directive.\n",
178+
explainDependency, from.String(), mod.Filepath, moduleName, mod.LineNumber)
179+
}
180+
continue MODULES_LOOP
172181
}
173182
} else {
174-
matches := ix.FindRulesByImportWithConfig(c, imp, languageName)
175-
if len(matches) == 0 {
176-
// Check if the imported module is part of the standard library.
177-
if isStd, err := isStdModule(mod); err != nil {
178-
log.Println("ERROR: ", err)
179-
hasFatalError = true
180-
continue MODULE_LOOP
181-
} else if isStd {
182-
continue MODULE_LOOP
183+
if dep, ok := cfg.FindThirdPartyDependency(moduleName); ok {
184+
deps.Add(dep)
185+
if explainDependency == dep {
186+
log.Printf("Explaining dependency (%s): "+
187+
"in the target %q, the file %q imports %q at line %d, "+
188+
"which resolves from the third-party module %q from the wheel %q.\n",
189+
explainDependency, from.String(), mod.Filepath, moduleName, mod.LineNumber, mod.Name, dep)
190+
}
191+
continue MODULES_LOOP
192+
} else {
193+
matches := ix.FindRulesByImportWithConfig(c, imp, languageName)
194+
if len(matches) == 0 {
195+
// Check if the imported module is part of the standard library.
196+
if isStd, err := isStdModule(module{Name: moduleName}); err != nil {
197+
log.Println("Error checking if standard module: ", err)
198+
hasFatalError = true
199+
continue POSSIBLE_MODULE_LOOP
200+
} else if isStd {
201+
continue MODULES_LOOP
202+
} else if cfg.ValidateImportStatements() {
203+
err := fmt.Errorf(
204+
"%[1]q at line %[2]d from %[3]q is an invalid dependency: possible solutions:\n"+
205+
"\t1. Add it as a dependency in the requirements.txt file.\n"+
206+
"\t2. Instruct Gazelle to resolve to a known dependency using the gazelle:resolve directive.\n"+
207+
"\t3. Ignore it with a comment '# gazelle:ignore %[1]s' in the Python file.\n",
208+
moduleName, mod.LineNumber, mod.Filepath,
209+
)
210+
errs = append(errs, err)
211+
continue POSSIBLE_MODULE_LOOP
212+
}
183213
}
184-
if cfg.ValidateImportStatements() {
185-
err := fmt.Errorf(
186-
"%[1]q at line %[2]d from %[3]q is an invalid dependency: possible solutions:\n"+
187-
"\t1. Add it as a dependency in the requirements.txt file.\n"+
188-
"\t2. Instruct Gazelle to resolve to a known dependency using the gazelle:resolve directive.\n"+
189-
"\t3. Ignore it with a comment '# gazelle:ignore %[1]s' in the Python file.\n",
190-
mod.Name, mod.LineNumber, mod.Filepath,
191-
)
192-
log.Printf("ERROR: failed to validate dependencies for target %q: %v\n", from.String(), err)
193-
hasFatalError = true
194-
continue MODULE_LOOP
214+
filteredMatches := make([]resolve.FindResult, 0, len(matches))
215+
for _, match := range matches {
216+
if match.IsSelfImport(from) {
217+
// Prevent from adding itself as a dependency.
218+
continue MODULES_LOOP
219+
}
220+
filteredMatches = append(filteredMatches, match)
195221
}
196-
}
197-
filteredMatches := make([]resolve.FindResult, 0, len(matches))
198-
for _, match := range matches {
199-
if match.IsSelfImport(from) {
200-
// Prevent from adding itself as a dependency.
201-
continue MODULE_LOOP
222+
if len(filteredMatches) == 0 {
223+
continue POSSIBLE_MODULE_LOOP
202224
}
203-
filteredMatches = append(filteredMatches, match)
204-
}
205-
if len(filteredMatches) == 0 {
206-
continue
207-
}
208-
if len(filteredMatches) > 1 {
209-
sameRootMatches := make([]resolve.FindResult, 0, len(filteredMatches))
210-
for _, match := range filteredMatches {
211-
if strings.HasPrefix(match.Label.Pkg, pythonProjectRoot) {
212-
sameRootMatches = append(sameRootMatches, match)
225+
if len(filteredMatches) > 1 {
226+
sameRootMatches := make([]resolve.FindResult, 0, len(filteredMatches))
227+
for _, match := range filteredMatches {
228+
if strings.HasPrefix(match.Label.Pkg, pythonProjectRoot) {
229+
sameRootMatches = append(sameRootMatches, match)
230+
}
231+
}
232+
if len(sameRootMatches) != 1 {
233+
err := fmt.Errorf(
234+
"multiple targets (%s) may be imported with %q at line %d in %q "+
235+
"- this must be fixed using the \"gazelle:resolve\" directive",
236+
targetListFromResults(filteredMatches), moduleName, mod.LineNumber, mod.Filepath)
237+
errs = append(errs, err)
238+
continue POSSIBLE_MODULE_LOOP
213239
}
240+
filteredMatches = sameRootMatches
214241
}
215-
if len(sameRootMatches) != 1 {
216-
err := fmt.Errorf(
217-
"multiple targets (%s) may be imported with %q at line %d in %q "+
218-
"- this must be fixed using the \"gazelle:resolve\" directive",
219-
targetListFromResults(filteredMatches), mod.Name, mod.LineNumber, mod.Filepath)
220-
log.Println("ERROR: ", err)
221-
hasFatalError = true
222-
continue MODULE_LOOP
242+
matchLabel := filteredMatches[0].Label.Rel(from.Repo, from.Pkg)
243+
dep := matchLabel.String()
244+
deps.Add(dep)
245+
if explainDependency == dep {
246+
log.Printf("Explaining dependency (%s): "+
247+
"in the target %q, the file %q imports %q at line %d, "+
248+
"which resolves from the first-party indexed labels.\n",
249+
explainDependency, from.String(), mod.Filepath, moduleName, mod.LineNumber)
223250
}
224-
filteredMatches = sameRootMatches
225-
}
226-
matchLabel := filteredMatches[0].Label.Rel(from.Repo, from.Pkg)
227-
dep := matchLabel.String()
228-
deps.Add(dep)
229-
if explainDependency == dep {
230-
log.Printf("Explaining dependency (%s): "+
231-
"in the target %q, the file %q imports %q at line %d, "+
232-
"which resolves from the first-party indexed labels.\n",
233-
explainDependency, from.String(), mod.Filepath, mod.Name, mod.LineNumber)
251+
continue MODULES_LOOP
234252
}
235253
}
254+
} // End possible modules loop.
255+
if len(errs) > 0 {
256+
// If, after trying all possible modules, we still haven't found anything, error out.
257+
log.Printf("Failed to find any valid modules from %v. Errors: %v\n", possibleModules, errs)
258+
hasFatalError = true
236259
}
237260
}
238261
if hasFatalError {

‎gazelle/testdata/from_imports/BUILD.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# gazelle:python_extension enabled

‎gazelle/testdata/from_imports/BUILD.out

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# gazelle:python_extension enabled

‎gazelle/testdata/from_imports/README.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# From Imports
2+
3+
This test case simulates imports of the form:
4+
5+
```python
6+
from foo import bar
7+
```

‎gazelle/testdata/from_imports/WORKSPACE

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# This is a test data Bazel workspace.

‎gazelle/testdata/from_imports/foo/BUILD.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+

‎gazelle/testdata/from_imports/foo/BUILD.out

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
load("@rules_python//python:defs.bzl", "py_library")
2+
3+
py_library(
4+
name = "foo",
5+
srcs = ["__init__.py"],
6+
imports = [".."],
7+
visibility = ["//:__subpackages__"],
8+
)

‎gazelle/testdata/from_imports/foo/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
foo = "foo"

‎gazelle/testdata/from_imports/foo/bar/BUILD.in

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
load("@rules_python//python:defs.bzl", "py_library")
2+
3+
# gazelle:python_ignore_files baz.py
4+
5+
py_library(
6+
name = "baz",
7+
srcs = [
8+
"baz.py",
9+
],
10+
imports = ["../.."],
11+
visibility = ["//:__subpackages__"],
12+
)
13+
14+
py_library(
15+
name = "bar",
16+
srcs = [
17+
"__init__.py",
18+
],
19+
imports = ["../.."],
20+
visibility = ["//:__subpackages__"],
21+
)

‎gazelle/testdata/from_imports/foo/bar/BUILD.out

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
load("@rules_python//python:defs.bzl", "py_library")
2+
3+
# gazelle:python_ignore_files baz.py
4+
5+
py_library(
6+
name = "baz",
7+
srcs = [
8+
"baz.py",
9+
],
10+
imports = ["../.."],
11+
visibility = ["//:__subpackages__"],
12+
)
13+
14+
py_library(
15+
name = "bar",
16+
srcs = [
17+
"__init__.py",
18+
],
19+
imports = ["../.."],
20+
visibility = ["//:__subpackages__"],
21+
)

‎gazelle/testdata/from_imports/foo/bar/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
bar = "bar"

‎gazelle/testdata/from_imports/foo/bar/baz.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
baz = "baz"

‎gazelle/testdata/from_imports/gazelle_python.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
manifest:
2+
modules_mapping:
3+
boto3: rootboto3
4+
boto4: rootboto4
5+
pip_deps_repository_name: root_pip_deps

‎gazelle/testdata/from_imports/import_from_init_py/BUILD.in

Whitespace-only changes.

‎gazelle/testdata/from_imports/import_from_init_py/BUILD.out

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
load("@rules_python//python:defs.bzl", "py_library")
2+
3+
py_library(
4+
name = "import_from_init_py",
5+
srcs = ["__init__.py"],
6+
imports = [".."],
7+
visibility = ["//:__subpackages__"],
8+
deps = ["//foo/bar"],
9+
)

‎gazelle/testdata/from_imports/import_from_init_py/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# bar is a variable inside foo/bar/__init__.py
2+
from foo.bar import bar

‎gazelle/testdata/from_imports/import_from_multiple/BUILD.in

Whitespace-only changes.

‎gazelle/testdata/from_imports/import_from_multiple/BUILD.out

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
load("@rules_python//python:defs.bzl", "py_library")
2+
3+
py_library(
4+
name = "import_from_multiple",
5+
srcs = ["__init__.py"],
6+
imports = [".."],
7+
visibility = ["//:__subpackages__"],
8+
deps = [
9+
"//foo/bar",
10+
"//foo/bar:baz",
11+
],
12+
)

‎gazelle/testdata/from_imports/import_from_multiple/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# Import multiple values from the same import.
2+
from foo.bar import bar, baz

‎gazelle/testdata/from_imports/import_nested_file/BUILD.in

Whitespace-only changes.

0 commit comments

Comments
 (0)