Skip to content
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

8.7.5: go.mod: adds official support for "replace" directive #585

Merged
merged 4 commits into from
Jan 8, 2024

Conversation

tiegz
Copy link
Contributor

@tiegz tiegz commented Jan 4, 2024

this should fix the replace parsing bugs, where we currently get {name: "foo -> bar", ...} back for deps:

  • adds parsing support for all go.mod directives: require, replace, extract, and retract
  • uses replace results to replace any matching deps in require results
  • ignore exclude results for now, since they're not related to require results

@tiegz tiegz force-pushed the tz/sc-38442/handle-modules-that-are-replaced-in-go-mod branch from 09ae6ac to 41fc122 Compare January 4, 2024 20:44
GOMOD_SINGLELINE_DEP_REGEX = /^(?<category>require|exclude|replace|retract)\s+#{GOMOD_DEP_REGEX}.*$/
GOMOD_MULTILINE_START_REGEX = /^(?<category>require|exclude|replace|retract)\s+\(/
GOMOD_MULTILINE_END_REGEX = /^\)/
GOMOD_MULTILINE_DEP_REGEX = /^#{GOMOD_DEP_REGEX}.*$/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

go.mod directives can be either singleline (require foo v1.0.0) or multiline (require (\n foo v1.0.0\n)) so this PR adds more intelligent parsing for both cases.

@tiegz tiegz changed the title go.mod: adds official support for "replace" directive 8.7.5: go.mod: adds official support for "replace" directive Jan 4, 2024
@tiegz tiegz force-pushed the tz/sc-38442/handle-modules-that-are-replaced-in-go-mod branch 3 times, most recently from 977f38c to b875d32 Compare January 4, 2024 21:20
@@ -7,8 +7,12 @@ class Go
include Bibliothecary::Analyser

GPM_REGEXP = /^(.+)\s+(.+)$/
GOMOD_REGEX = /^(require\s+)?(.+)\s+(.+)$/
GOMOD_IGNORABLE_REGEX = /^(\/\/|module\s|go\s|exclude\s|replace\s|require\s+\(|\))/m
GOMOD_REPLACEMENT_SEPARATOR_REGEX = / => /
Copy link
Contributor

Choose a reason for hiding this comment

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

I was not sure what I was looking at initially. WDYT about using the %r syntax here to make it more clear this is a regular expression rather than...two divisions with an arrow operator?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figured it'd be clear since the var is suffixed with _REGEX, forward-slashes are the ubiquitous delimiter for regexes and it's listed above/below the other regexes, but I'm not opposed to using a different syntax here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually, I'm hesitant to start mixing syntaxes in bibliothecary, so I'll just replace the whitespaces with \s to make it more obvious

@@ -163,6 +194,34 @@ def self.map_dependencies(manifest, attr_name, dep_attr_name, version_attr_name,
}
end
end

def self.go_mod_category_relative_dep(category, line, match)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this use named parameters instead?

current_multiline_category = match[1]
elsif (match = line.match(GOMOD_SINGLELINE_DEP_REGEX)) # or, detect a singleline dep
categorized_deps[match[:category]] << go_mod_category_relative_dep(match[:category], line, match)
elsif (current_multiline_category && match = line.gsub(/(\/\/(.*))/, "").match(GOMOD_MULTILINE_DEP_REGEX)) # otherwise, parse the multiline dep
Copy link
Contributor

Choose a reason for hiding this comment

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

This gsub is for removing comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I should probably extract that into a constant too

@tiegz tiegz force-pushed the tz/sc-38442/handle-modules-that-are-replaced-in-go-mod branch from 0a78675 to 6b446e5 Compare January 5, 2024 22:50
@tiegz tiegz force-pushed the tz/sc-38442/handle-modules-that-are-replaced-in-go-mod branch from 6b446e5 to 5394610 Compare January 5, 2024 22:51
@tiegz tiegz requested a review from tradiff January 5, 2024 22:55
@tiegz tiegz self-assigned this Jan 8, 2024
@tiegz tiegz merged commit 30e1db0 into main Jan 8, 2024
2 checks passed
@tiegz tiegz deleted the tz/sc-38442/handle-modules-that-are-replaced-in-go-mod branch January 8, 2024 15:29
andrew pushed a commit to ecosyste-ms/bibliothecary that referenced this pull request Jan 8, 2024
…esio#585)

* go.mod: adds official support for "replace" directive

* 8.7.5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants