-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Avoid discarding file-like arguments in dotnet file.cs invocations
#52110
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
Conversation
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.
Pull request overview
This PR fixes a bug where file-like arguments were incorrectly discarded when the same filename appeared both as the entry point and as a program argument (e.g., dotnet Program.cs Program.cs). The fix changes the token filtering logic from value-based comparison to reference-based comparison, ensuring only the first occurrence (the entry point) is removed while preserving subsequent occurrences as arguments.
Key changes:
- Modified
TryRunFileBasedAppto capture the actual token object rather than just its value - Changed token filtering to use reference equality instead of value equality
- Added a test case specifically for the bug scenario (
dotnet Program.cs Program.cs)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Cli/dotnet/Program.cs | Fixed token filtering logic in TryRunFileBasedApp to use reference equality, preventing all file-like arguments from being discarded when they match the entry point filename |
| test/dotnet.Tests/CommandTests/Run/RunFileTests.cs | Added test case verifying that dotnet Program.cs Program.cs correctly passes "Program.cs" as an argument to the executed program |
| foreach (var token in parseResult.Tokens) | ||
| { | ||
| if (token.Type != TokenType.Argument || token.Value != unmatchedCommandOrFile) | ||
| if (token != unmatchedCommandOrFile) |
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.
There isn't a need to make a further change, but, I was wondering if otherTokens ends up being equivalent to parseResult.Tokens[1..]? (I guess with some intermediate .Value access inserted.)
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.
I don't think that's guaranteed, there might be some special tokens before the subcommand argument. But I'm also not sure it makes sense to try to preserve the order until we have a motivating scenario.
|
/ba-g known failures |
Fixes #52108.