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

x/mod/modfile: scan ASCII bracket characters and commas as separate tokens #38167

Closed
jayconrod opened this issue Mar 30, 2020 · 3 comments
Closed
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@jayconrod
Copy link
Contributor

The go.mod parser treats the characters ( and ) as separate tokens if they appear after whitespace characters or after the end of another token. They are not treated as separate tokens if they appear after other non-whitespace characters though.

require()  // scanned as 1 token:  "require()"
require () // scanned as 3 tokens: "require" "(" ")"

This is making it difficult to add new syntax, such as the version ranges suggested in #24031. It's also a problem that parentheses are the only recognized bracket character.

retract (v1.0.0, v1.1.0) // scanned as 4 tokens: "retract" "(" "v1.0.0," "v1.1.0)"
retract [v1.0.0, v1.1.0] // scanned as 3 tokens: "retract" "[v1.0.0," "v1.10]"

Ideally, the bracket characters (( ) [ ] { }) and the comma (,) would be scanned as separate characters when unquoted, regardless of surrounding whitespace characters.

retract [v1.0.0, v1.1.0] // should be scanned as 6 tokens: "retract" "[" "v1.0.0" "," "v1.1.0" "]"

None of these characters are allowed in module paths or versions, but they are allowed in file paths and may appear on the right side of a replace directive. So for the sake of compatibility, we should make this change in two steps:

  • In Go 1.15:
    • The characters [ ] { } , should be scanned as separate tokens when preceded by whitespace. These are not valid at the beginning of any token today (including file paths), so this should be safe to do immediately. The characters ( ) are already scanned this way.
    • For directives added after 1.14 (i.e., retract), the characters ( ) [ ] { } , are scanned as separate tokens, even when preceded by non-whitespace characters in all directives. Old versions of the go command don't need to scan these directives correctly.
    • Tokens that will be scanned differently in the future will be quoted whenever go.mod is formatted. So the path ./foo() will be automatically rewritten as "./foo()".
  • In Go 1.17 (when 1.15 is no longer supported):
    • For all directives, the characters ( ) [ ] { } , are scanned as separate tokens, even when preceded by non-whitespace characters in all directives.

Blocks #24031
Related #38144

cc @bcmills @matloob @rsc

@jayconrod jayconrod added NeedsFix The path to resolution is known, but the work has not been done. GoCommand cmd/go modules labels Mar 30, 2020
@jayconrod jayconrod added this to the Go1.15 milestone Mar 30, 2020
@jayconrod jayconrod self-assigned this Mar 30, 2020
@jayconrod
Copy link
Contributor Author

Another change needed:

  • ( should only start a block if there are no more tokens on the line.
  • ) should only end a block if it's the first token on the line.
  • As a special case, ( followed immediately by ) is an empty block.

@jayconrod
Copy link
Contributor Author

After discussing this with @bcmills and @matloob, it's probably okay if we break a small number of replace file paths. replace directives outside the main module are ignored by modfile.ParseLax. So they might be tokenized incorrectly, but the go command won't notice.

So it looks like we can do this all in one step.

@gopherbot
Copy link

Change https://golang.org/cl/226639 mentions this issue: modfile: scan ASCII brackets and commas as separate tokens

@golang golang locked and limited conversation to collaborators Apr 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

2 participants