Skip to content

Resolve dependencies when imports are relative to the package path #2865

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion gazelle/python/file_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,9 @@ func (p *FileParser) parseImportStatements(node *sitter.Node) bool {
}
} else if node.Type() == sitterNodeTypeImportFromStatement {
from := node.Child(1).Content(p.code)
if strings.HasPrefix(from, ".") {
// If the import is from the current package, we don't need to add it to the modules i.e. from . import Class1.
// If the import is from a different relative package i.e. from .package1 import foo, we need to add it to the modules.
if from == "." {
return true
}
for j := 3; j < int(node.ChildCount()); j++ {
Expand Down
48 changes: 46 additions & 2 deletions gazelle/python/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,56 @@ func (py *Resolver) Resolve(
modules := modulesRaw.(*treeset.Set)
it := modules.Iterator()
explainDependency := os.Getenv("EXPLAIN_DEPENDENCY")
// Resolve relative paths for package generation
isPackageGeneration := !cfg.PerFileGeneration() && !cfg.CoarseGrainedGeneration()
hasFatalError := false
MODULES_LOOP:
for it.Next() {
mod := it.Value().(module)
moduleParts := strings.Split(mod.Name, ".")
possibleModules := []string{mod.Name}
moduleName := mod.Name
// Transform relative imports `.` or `..foo.bar` into the package path from root.
if strings.HasPrefix(moduleName, ".") {
// If not package generation mode, skip relative imports
if !isPackageGeneration {
continue MODULES_LOOP
}
relativeDepth := 0
for i := 0; i < len(moduleName); i++ {
if moduleName[i] == '.' {
relativeDepth++
} else {
break
}
}

// Extract suffix after leading dots
relativeSuffix := moduleName[relativeDepth:]
var relativeSuffixParts []string
if relativeSuffix != "" {
relativeSuffixParts = strings.Split(relativeSuffix, ".")
}

// Split current package label into parts
pkgParts := strings.Split(from.Pkg, "/")

if relativeDepth- 1 > len(pkgParts) {
// Trying to go above the root
log.Printf("ERROR: Invalid relative import %q in %q: exceeds package root.", moduleName, mod.Filepath)
continue MODULES_LOOP
}

// Go up `relativeDepth - 1` levels
baseParts := pkgParts
if relativeDepth > 1 {
baseParts = pkgParts[:len(pkgParts)-(relativeDepth-1)]
}

absParts := append(baseParts, relativeSuffixParts...)
moduleName = strings.Join(absParts, ".")
}

moduleParts := strings.Split(moduleName, ".")
possibleModules := []string{moduleName}
for len(moduleParts) > 1 {
// Iterate back through the possible imports until
// a match is found.
Expand Down
13 changes: 2 additions & 11 deletions gazelle/python/testdata/relative_imports/BUILD.out
Original file line number Diff line number Diff line change
@@ -1,23 +1,14 @@
load("@rules_python//python:defs.bzl", "py_binary", "py_library")
load("@rules_python//python:defs.bzl", "py_binary")

# gazelle:resolve py resolved_package //package2:resolved_package

py_library(
name = "relative_imports",
srcs = [
"package1/module1.py",
"package1/module2.py",
],
visibility = ["//:__subpackages__"],
)

py_binary(
name = "relative_imports_bin",
srcs = ["__main__.py"],
main = "__main__.py",
visibility = ["//:__subpackages__"],
deps = [
":relative_imports",
"//package1",
"//package2",
],
)
Empty file.
10 changes: 10 additions & 0 deletions gazelle/python/testdata/relative_imports/package1/BUILD.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
load("@rules_python//python:defs.bzl", "py_library")

py_library(
name = "package1",
srcs = [
"module1.py",
"module2.py",
],
visibility = ["//:__subpackages__"],
)
6 changes: 4 additions & 2 deletions gazelle/python/testdata/relative_imports/package2/BUILD.out
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ py_library(
"__init__.py",
"module3.py",
"module4.py",
"subpackage1/module5.py",
],
visibility = ["//:__subpackages__"],
deps = [":resolved_package"],
deps = [
":resolved_package",
"//package2/subpackage1",
],
)
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
load("@rules_python//python:defs.bzl", "py_library")

py_library(
name = "subpackage1",
srcs = ["module5.py"],
visibility = ["//:__subpackages__"],
deps = ["//package2"],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# gazelle:python_generation_mode package
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job in covering package mode. Can we also have test cases covering file and project mode?

It seems "testdata/relative_imports" and "testdata/reslove_deps_relative_imports" are covering similar things. From the name, it's also hard to tell the difference between "testdata/relative_imports" and "testdata/reslove_deps_relative_imports"

We can either:

  1. have all test cases in "testdata/relative_imports", with some packages in package mode, some in project mode and some in file mode, or
  2. have "testdata/relative_imports_package_mode", "testdata/relative_imports_file_mode" and "testdata/relative_imports_project_mode"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 2 is preferrable? Seems more straight forward and the behavior would not change in project mode or file mode.
Is file mode also something that we want to look into supporting too? I think if we do, then implementations for relative paths in file mode should be in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can support file mode in the next PR.

14 changes: 14 additions & 0 deletions gazelle/python/testdata/resolve_deps_relative_imports/BUILD.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
load("@rules_python//python:defs.bzl", "py_binary")

# gazelle:python_generation_mode package

py_binary(
name = "resolve_deps_relative_imports_bin",
srcs = ["__main__.py"],
main = "__main__.py",
visibility = ["//:__subpackages__"],
deps = [
"//package1",
"//package2",
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Resolve deps for relative imports

This test case verifies that the generated targets correctly handle relative imports in Python. Specifically, when the Python generation mode is set to "package," it ensures that relative import statements such as from .foo import X are properly resolved to their corresponding modules.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# This is a test data Bazel workspace.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from package1.module1 import function1
from package2.module3 import function3

print(function1())
print(function3())
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
load("@rules_python//python:defs.bzl", "py_library")

py_library(
name = "package1",
srcs = [
"module1.py",
"module2.py",
],
visibility = ["//:__subpackages__"],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Copyright 2023 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from .module2 import function2


def function1():
return "function1 " + function2()
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Copyright 2023 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.


def function2():
return "function2"
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
load("@rules_python//python:defs.bzl", "py_library")

py_library(
name = "package2",
srcs = [
"__init__.py",
"module3.py",
"module4.py",
],
visibility = ["//:__subpackages__"],
deps = ["//package2/library"],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
from .library import add as _add
from .library import divide as _divide
from .library import multiply as _multiply
from .library import subtract as _subtract


def add(a, b):
return _add(a, b)


def divide(a, b):
return _divide(a, b)


def multiply(a, b):
return _multiply(a, b)


def subtract(a, b):
return _subtract(a, b)
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
load("@rules_python//python:defs.bzl", "py_library")

py_library(
name = "library",
srcs = ["__init__.py"],
visibility = ["//:__subpackages__"],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
def add(a, b):
return a + b


def divide(a, b):
return a / b


def multiply(a, b):
return a * b


def subtract(a, b):
return a - b
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from .library import function5


def function3():
return "function3 " + function5()
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def function4():
return "function4"
15 changes: 15 additions & 0 deletions gazelle/python/testdata/resolve_deps_relative_imports/test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Copyright 2023 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

---