-
-
Notifications
You must be signed in to change notification settings - Fork 665
Add support for replace directive in go.mod files
#4673
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
base: develop
Are you sure you want to change the base?
Changes from all commits
d478275
27c9184
ba1a48e
d52b301
21d1091
2108013
c7192a5
9889074
01498c0
4d40657
d6a0252
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -21,6 +21,7 @@ class GoModule(object): | |||||||||||||||||||||||||||||||||||||
| module = attr.ib(default=None) | ||||||||||||||||||||||||||||||||||||||
| require = attr.ib(default=None) | ||||||||||||||||||||||||||||||||||||||
| exclude = attr.ib(default=None) | ||||||||||||||||||||||||||||||||||||||
| local_replacements = attr.ib(default=None) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| def purl(self, include_version=True): | ||||||||||||||||||||||||||||||||||||||
| version = None | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -50,6 +51,13 @@ def purl(self, include_version=True): | |||||||||||||||||||||||||||||||||||||
| r'(?P<version>(.*))' | ||||||||||||||||||||||||||||||||||||||
| ).match | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| parse_rep_link = re.compile( | ||||||||||||||||||||||||||||||||||||||
| r"(?P<ns_name>\S+)" | ||||||||||||||||||||||||||||||||||||||
| r"(?:\s+(?P<version>\S+))?" | ||||||||||||||||||||||||||||||||||||||
| r"\s*=>\s*" | ||||||||||||||||||||||||||||||||||||||
| r"(?P<replacement_ns_name>\S+)" | ||||||||||||||||||||||||||||||||||||||
| r"(?:\s+(?P<replacement_version>\S+))?" | ||||||||||||||||||||||||||||||||||||||
| ).match | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| def preprocess(line): | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -60,6 +68,60 @@ def preprocess(line): | |||||||||||||||||||||||||||||||||||||
| line = line.strip() | ||||||||||||||||||||||||||||||||||||||
| return line | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| def parse_replace_directive(line): | ||||||||||||||||||||||||||||||||||||||
| parsed_replace = parse_rep_link(line) | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
| parsed_replace = parse_rep_link(line) | |
| parsed_replace = parse_rep_link(line) | |
| if not parsed_replace: | |
| return None, None |
Copilot
AI
Jan 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a module path doesn't contain any slashes (e.g., a simple module name), rpartition('/') will return ('', '', 'modulename'), resulting in an empty namespace and the full name in the name field. This is the expected behavior for single-component module names, but consider adding a comment to clarify this edge case handling.
| version = parsed_replace.group("version") | |
| version = parsed_replace.group("version") | |
| # When ns_name has no '/', rpartition('/') returns ('', '', ns_name), | |
| # resulting in an empty namespace and the full module name in `name`. | |
| # This is the expected behavior for single-component module names. |
Copilot
AI
Jan 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When namespace is empty (e.g., for single-word modules without slashes), the replaces string will start with a slash like "/modulename". This happens when original.get('namespace') returns an empty string. Consider handling this case to avoid creating malformed module paths.
| local_replacements.append({ | |
| 'replaces': f"{original.get('namespace')}/{original.get('name')}", | |
| original_namespace = original.get('namespace') or '' | |
| original_name = original.get('name') or '' | |
| if original_namespace: | |
| replaces = f"{original_namespace}/{original_name}" | |
| else: | |
| replaces = original_name | |
| local_replacements.append({ | |
| 'replaces': replaces, |
Copilot
AI
Jan 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The block-style parsing doesn't handle empty lines or check if the regex match succeeds before calling handle_replace_directive. Empty lines within the block or lines that don't match the expected format will cause failures. Add checks to skip empty lines and validate the match before processing.
| break | |
| handle_replace_directive(rep, require, exclude, local_replacements) | |
| continue | |
| if 'replace' in line and '=>' in line: | |
| line = line.lstrip("replace").strip() | |
| break | |
| # Skip empty or non-directive lines inside a replace block | |
| if not rep or '=>' not in rep: | |
| continue | |
| handle_replace_directive(rep, require, exclude, local_replacements) | |
| continue | |
| if 'replace' in line and '=>' in line: | |
| line = line.lstrip("replace").strip() | |
| # Ensure we only process non-empty replace directives | |
| if not line: | |
| continue |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| module golang.org/x/tools/gopls | ||
|
|
||
| go 1.18 | ||
|
|
||
| require ( | ||
| github.com/google/go-cmp v0.5.9 | ||
| github.com/jba/printsrc v0.2.2 | ||
| github.com/jba/templatecheck v0.6.0 | ||
| github.com/sergi/go-diff v1.1.0 | ||
| golang.org/x/mod v0.12.0 | ||
| golang.org/x/sync v0.3.0 | ||
| golang.org/x/sys v0.11.0 | ||
| golang.org/x/telemetry v0.0.0-20230808152233-a65b40c0fdb0 | ||
| golang.org/x/text v0.12.0 | ||
| golang.org/x/tools v0.6.0 | ||
| golang.org/x/vuln v0.0.0-20230110180137-6ad3e3d07815 | ||
| gopkg.in/yaml.v3 v3.0.1 | ||
| honnef.co/go/tools v0.4.2 | ||
| mvdan.cc/gofumpt v0.4.0 | ||
| mvdan.cc/xurls/v2 v2.4.0 | ||
| ) | ||
|
|
||
| require ( | ||
| github.com/BurntSushi/toml v1.2.1 // indirect | ||
| github.com/google/safehtml v0.1.0 // indirect | ||
| golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e // indirect | ||
| golang.org/x/exp/typeparams v0.0.0-20221212164502-fae10dda9338 // indirect | ||
| ) | ||
|
|
||
| replace golang.org/x/tools => ../ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring is minimal and doesn't explain the behavior for different types of replace directives (local vs. remote, with/without versions). Consider adding examples and explaining the return value structure for better maintainability.