709/from imports

gazelle/parse.py

@@ -21,15 +21,18 @@ def parse_import_statements(content, filepath):
                     "name": subnode.name,
                     "lineno": node.lineno,
                     "filepath": filepath,
+                    "from": ""
                 }
                 modules.append(module)
         elif isinstance(node, ast.ImportFrom) and node.level == 0:
-            module = {
-                "name": node.module,
-                "lineno": node.lineno,
-                "filepath": filepath,
-            }
-            modules.append(module)
+            for subnode in node.names:
+                module = {
+                    "name": f"{node.module}.{subnode.name}",
+                    "lineno": node.lineno,
+                    "filepath": filepath,
+                    "from": node.module
+                }
+                modules.append(module)
     return modules
 
 
@@ -47,9 +50,10 @@ def parse(repo_root, rel_package_path, filename):
     abs_filepath = os.path.join(repo_root, rel_filepath)
     with open(abs_filepath, "r") as file:
         content = file.read()
-       # From simple benchmarks, 2 workers gave the best performance here.
+        # From simple benchmarks, 2 workers gave the best performance here.
         with concurrent.futures.ThreadPoolExecutor(max_workers=2) as executor:
-            modules_future = executor.submit(parse_import_statements, content, rel_filepath)
+            modules_future = executor.submit(parse_import_statements, content,
+                                             rel_filepath)
             comments_future = executor.submit(parse_comments, content)
         modules = modules_future.result()
         comments = comments_future.result()
@@ -69,11 +73,12 @@ def main(stdin, stdout):
             filenames = parse_request["filenames"]
             outputs = list()
             if len(filenames) == 1:
-                outputs.append(parse(repo_root, rel_package_path, filenames[0]))
+                outputs.append(parse(repo_root, rel_package_path,
+                                     filenames[0]))
             else:
                 futures = [
-                    executor.submit(parse, repo_root, rel_package_path, filename)
-                    for filename in filenames
+                    executor.submit(parse, repo_root, rel_package_path,
+                                    filename) for filename in filenames
                     if filename != ""
                 ]
                 for future in concurrent.futures.as_completed(futures):

gazelle/parser.go

@@ -133,13 +133,13 @@ func (p *python3Parser) parse(pyFilenames *treeset.Set) (*treeset.Set, error) {
 		for _, m := range res.Modules {
 			// Check for ignored dependencies set via an annotation to the Python
 			// module.
-			if annotations.ignores(m.Name) {
+			if annotations.ignores(m.Name) || annotations.ignores(m.From) {
 				continue
 			}
 
 			// Check for ignored dependencies set via a Gazelle directive in a BUILD
 			// file.
-			if p.ignoresDependency(m.Name) {
+			if p.ignoresDependency(m.Name) || p.ignoresDependency(m.From) {
 				continue
 			}
 
@@ -170,6 +170,9 @@ type module struct {
 	LineNumber uint32 `json:"lineno"`
 	// The path to the module file relative to the Bazel workspace root.
 	Filepath string `json:"filepath"`
+	// If this was a from import, e.g. from foo import bar, From indicates the module
+	// from which it is imported.
+	From string `json:"from"`
 }
 
 // moduleComparator compares modules by name.

gazelle/resolve.go

@@ -140,99 +140,126 @@ func (py *Resolver) Resolve(
 		it := modules.Iterator()
 		explainDependency := os.Getenv("EXPLAIN_DEPENDENCY")
 		hasFatalError := false
-	MODULE_LOOP:
+	MODULES_LOOP:
 		for it.Next() {
 			mod := it.Value().(module)
-			imp := resolve.ImportSpec{Lang: languageName, Imp: mod.Name}
-			if override, ok := resolve.FindRuleWithOverride(c, imp, languageName); ok {
-				if override.Repo == "" {
-					override.Repo = from.Repo
-				}
-				if !override.Equal(from) {
-					if override.Repo == from.Repo {
-						override.Repo = ""
-					}
-					dep := override.String()
-					deps.Add(dep)
-					if explainDependency == dep {
-						log.Printf("Explaining dependency (%s): "+
-							"in the target %q, the file %q imports %q at line %d, "+
-							"which resolves using the \"gazelle:resolve\" directive.\n",
-							explainDependency, from.String(), mod.Filepath, mod.Name, mod.LineNumber)
+			moduleParts := strings.Split(mod.Name, ".")
+			possibleModules := []string{mod.Name}
+			for len(moduleParts) > 1 {
+				// Iterate back through the possible imports until
+				// a match is found.
+				// For example, "from foo.bar import baz" where bar is a variable, we should try
+				// `foo.bar.baz` first, then `foo.bar`, then `foo`. In the first case, the import could be file `baz.py`
+				// in the directory `foo/bar`.
+				// Or, the import could be variable `bar` in file `foo/bar.py`.
+				// The import could also be from a standard module, e.g. `six.moves`, where
+				// the dependency is actually `six`.
+				moduleParts = moduleParts[:len(moduleParts)-1]
+				possibleModules = append(possibleModules, strings.Join(moduleParts, "."))
+			}
+			errs := []error{}
+		POSSIBLE_MODULE_LOOP:
+			for _, moduleName := range possibleModules {
+				imp := resolve.ImportSpec{Lang: languageName, Imp: moduleName}
+				if override, ok := resolve.FindRuleWithOverride(c, imp, languageName); ok {
+					if override.Repo == "" {
+						override.Repo = from.Repo
 					}
-				}
-			} else {
-				if dep, ok := cfg.FindThirdPartyDependency(mod.Name); ok {
-					deps.Add(dep)
-					if explainDependency == dep {
-						log.Printf("Explaining dependency (%s): "+
-							"in the target %q, the file %q imports %q at line %d, "+
-							"which resolves from the third-party module %q from the wheel %q.\n",
-							explainDependency, from.String(), mod.Filepath, mod.Name, mod.LineNumber, mod.Name, dep)
+					if !override.Equal(from) {
+						if override.Repo == from.Repo {
+							override.Repo = ""
+						}
+						dep := override.String()
+						deps.Add(dep)
+						if explainDependency == dep {
+							log.Printf("Explaining dependency (%s): "+
+								"in the target %q, the file %q imports %q at line %d, "+
+								"which resolves using the \"gazelle:resolve\" directive.\n",
+								explainDependency, from.String(), mod.Filepath, moduleName, mod.LineNumber)
+						}
+						continue MODULES_LOOP
 					}
 				} else {
-					matches := ix.FindRulesByImportWithConfig(c, imp, languageName)
-					if len(matches) == 0 {
-						// Check if the imported module is part of the standard library.
-						if isStd, err := isStdModule(mod); err != nil {
-							log.Println("ERROR: ", err)
-							hasFatalError = true
-							continue MODULE_LOOP
-						} else if isStd {
-							continue MODULE_LOOP
+					if dep, ok := cfg.FindThirdPartyDependency(moduleName); ok {
+						deps.Add(dep)
+						if explainDependency == dep {
+							log.Printf("Explaining dependency (%s): "+
+								"in the target %q, the file %q imports %q at line %d, "+
+								"which resolves from the third-party module %q from the wheel %q.\n",
+								explainDependency, from.String(), mod.Filepath, moduleName, mod.LineNumber, mod.Name, dep)
 						}
-						if cfg.ValidateImportStatements() {
-							err := fmt.Errorf(
-								"%[1]q at line %[2]d from %[3]q is an invalid dependency: possible solutions:\n"+
-									"\t1. Add it as a dependency in the requirements.txt file.\n"+
-									"\t2. Instruct Gazelle to resolve to a known dependency using the gazelle:resolve directive.\n"+
-									"\t3. Ignore it with a comment '# gazelle:ignore %[1]s' in the Python file.\n",
-								mod.Name, mod.LineNumber, mod.Filepath,
-							)
-							log.Printf("ERROR: failed to validate dependencies for target %q: %v\n", from.String(), err)
-							hasFatalError = true
-							continue MODULE_LOOP
+						continue MODULES_LOOP
+					} else {
+						matches := ix.FindRulesByImportWithConfig(c, imp, languageName)
+						if len(matches) == 0 {
+							// Check if the imported module is part of the standard library.
+							if isStd, err := isStdModule(module{Name: moduleName}); err != nil {
+								log.Println("Error checking if standard module: ", err)
+								hasFatalError = true
+								continue POSSIBLE_MODULE_LOOP
+							} else if isStd {
+								continue MODULES_LOOP
+							} else if cfg.ValidateImportStatements() {
+								err := fmt.Errorf(
+									"%[1]q at line %[2]d from %[3]q is an invalid dependency: possible solutions:\n"+
+										"\t1. Add it as a dependency in the requirements.txt file.\n"+
+										"\t2. Instruct Gazelle to resolve to a known dependency using the gazelle:resolve directive.\n"+
+										"\t3. Ignore it with a comment '# gazelle:ignore %[1]s' in the Python file.\n",
+									moduleName, mod.LineNumber, mod.Filepath,
+								)
+								errs = append(errs, err)
+								continue POSSIBLE_MODULE_LOOP
+							}
 						}
-					}
-					filteredMatches := make([]resolve.FindResult, 0, len(matches))
-					for _, match := range matches {
-						if match.IsSelfImport(from) {
-							// Prevent from adding itself as a dependency.
-							continue MODULE_LOOP
+						filteredMatches := make([]resolve.FindResult, 0, len(matches))
+						for _, match := range matches {
+							if match.IsSelfImport(from) {
+								// Prevent from adding itself as a dependency.
+								continue MODULES_LOOP
+							}
+							filteredMatches = append(filteredMatches, match)
 						}
-						filteredMatches = append(filteredMatches, match)
-					}
-					if len(filteredMatches) == 0 {
-						continue
-					}
-					if len(filteredMatches) > 1 {
-						sameRootMatches := make([]resolve.FindResult, 0, len(filteredMatches))
-						for _, match := range filteredMatches {
-							if strings.HasPrefix(match.Label.Pkg, pythonProjectRoot) {
-								sameRootMatches = append(sameRootMatches, match)
+						if len(filteredMatches) == 0 {
+							continue POSSIBLE_MODULE_LOOP
+						}
+						if len(filteredMatches) > 1 {
+							sameRootMatches := make([]resolve.FindResult, 0, len(filteredMatches))
+							for _, match := range filteredMatches {
+								if strings.HasPrefix(match.Label.Pkg, pythonProjectRoot) {
+									sameRootMatches = append(sameRootMatches, match)
+								}
 							}
+							if len(sameRootMatches) != 1 {
+								err := fmt.Errorf(
+									"multiple targets (%s) may be imported with %q at line %d in %q "+
+										"- this must be fixed using the \"gazelle:resolve\" directive",
+									targetListFromResults(filteredMatches), moduleName, mod.LineNumber, mod.Filepath)
+								errs = append(errs, err)
+								continue POSSIBLE_MODULE_LOOP
+							}
+							filteredMatches = sameRootMatches
 						}
-						if len(sameRootMatches) != 1 {
-							err := fmt.Errorf(
-								"multiple targets (%s) may be imported with %q at line %d in %q "+
-									"- this must be fixed using the \"gazelle:resolve\" directive",
-								targetListFromResults(filteredMatches), mod.Name, mod.LineNumber, mod.Filepath)
-							log.Println("ERROR: ", err)
-							hasFatalError = true
-							continue MODULE_LOOP
+						matchLabel := filteredMatches[0].Label.Rel(from.Repo, from.Pkg)
+						dep := matchLabel.String()
+						deps.Add(dep)
+						if explainDependency == dep {
+							log.Printf("Explaining dependency (%s): "+
+								"in the target %q, the file %q imports %q at line %d, "+
+								"which resolves from the first-party indexed labels.\n",
+								explainDependency, from.String(), mod.Filepath, moduleName, mod.LineNumber)
 						}
-						filteredMatches = sameRootMatches
-					}
-					matchLabel := filteredMatches[0].Label.Rel(from.Repo, from.Pkg)
-					dep := matchLabel.String()
-					deps.Add(dep)
-					if explainDependency == dep {
-						log.Printf("Explaining dependency (%s): "+
-							"in the target %q, the file %q imports %q at line %d, "+
-							"which resolves from the first-party indexed labels.\n",
-							explainDependency, from.String(), mod.Filepath, mod.Name, mod.LineNumber)
+						continue MODULES_LOOP
 					}
 				}
+			} // End possible modules loop.
+			if len(errs) > 0 {
+				// If, after trying all possible modules, we still haven't found anything, error out.
+				joinedErrs := ""
+				for _, err := range errs {
+					joinedErrs = fmt.Sprintf("%s%s\n", joinedErrs, err)
+				}
+				log.Printf("ERROR: failed to validate dependencies for target %q: %v\n", from.String(), joinedErrs)
+				hasFatalError = true
 			}
 		}
 		if hasFatalError {

gazelle/testdata/from_imports/BUILD.in

@@ -0,0 +1 @@
+# gazelle:python_extension enabled

gazelle/testdata/from_imports/BUILD.out

@@ -0,0 +1 @@
+# gazelle:python_extension enabled

gazelle/testdata/from_imports/README.md

@@ -0,0 +1,7 @@
+# From Imports
+
+This test case simulates imports of the form:
+
+```python
+from foo import bar
+```

gazelle/testdata/from_imports/WORKSPACE

@@ -0,0 +1 @@
+# This is a test data Bazel workspace.

gazelle/testdata/from_imports/foo/BUILD.in

@@ -0,0 +1 @@
+

gazelle/testdata/from_imports/foo/BUILD.out

@@ -0,0 +1,8 @@
+load("@rules_python//python:defs.bzl", "py_library")
+
+py_library(
+    name = "foo",
+    srcs = ["__init__.py"],
+    imports = [".."],
+    visibility = ["//:__subpackages__"],
+)

gazelle/testdata/from_imports/foo/__init__.py

@@ -0,0 +1 @@
+foo = "foo"

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

@@ -0,0 +1,21 @@
+load("@rules_python//python:defs.bzl", "py_library")
+
+# gazelle:python_ignore_files baz.py
+
+py_library(
+    name = "baz",
+    srcs = [
+        "baz.py",
+    ],
+    imports = ["../.."],
+    visibility = ["//:__subpackages__"],
+)
+
+py_library(
+    name = "bar",
+    srcs = [
+        "__init__.py",
+    ],
+    imports = ["../.."],
+    visibility = ["//:__subpackages__"],
+)

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

@@ -0,0 +1,21 @@
+load("@rules_python//python:defs.bzl", "py_library")
+
+# gazelle:python_ignore_files baz.py
+
+py_library(
+    name = "baz",
+    srcs = [
+        "baz.py",
+    ],
+    imports = ["../.."],
+    visibility = ["//:__subpackages__"],
+)
+
+py_library(
+    name = "bar",
+    srcs = [
+        "__init__.py",
+    ],
+    imports = ["../.."],
+    visibility = ["//:__subpackages__"],
+)

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

@@ -0,0 +1 @@
+bar = "bar"

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

@@ -0,0 +1 @@
+baz = "baz"

gazelle/testdata/from_imports/gazelle_python.yaml

@@ -0,0 +1,5 @@
+manifest:
+  modules_mapping:
+    boto3: rootboto3
+    boto4: rootboto4
+  pip_deps_repository_name: root_pip_deps

gazelle/testdata/from_imports/import_from_init_py/BUILD.out

@@ -0,0 +1,9 @@
+load("@rules_python//python:defs.bzl", "py_library")
+
+py_library(
+    name = "import_from_init_py",
+    srcs = ["__init__.py"],
+    imports = [".."],
+    visibility = ["//:__subpackages__"],
+    deps = ["//foo/bar"],
+)

gazelle/testdata/from_imports/import_from_init_py/__init__.py

@@ -0,0 +1,2 @@
+# bar is a variable inside foo/bar/__init__.py
+from foo.bar import bar

gazelle/testdata/from_imports/import_from_multiple/BUILD.out

@@ -0,0 +1,12 @@
+load("@rules_python//python:defs.bzl", "py_library")
+
+py_library(
+    name = "import_from_multiple",
+    srcs = ["__init__.py"],
+    imports = [".."],
+    visibility = ["//:__subpackages__"],
+    deps = [
+        "//foo/bar",
+        "//foo/bar:baz",
+    ],
+)