Skip to content

Disallow duplicate file-level directives #49308

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

Merged
merged 3 commits into from
Jun 11, 2025

Conversation

jjonescz
Copy link
Member

Resolves #48842.

@jjonescz jjonescz requested a review from a team June 10, 2025 07:16
@jjonescz jjonescz added the Area-run-file Items related to the "dotnet run <file>" effort label Jun 10, 2025
@jjonescz jjonescz marked this pull request as ready for review June 10, 2025 12:02
@Copilot Copilot AI review requested due to automatic review settings June 10, 2025 12:02
Copy link
Contributor

@Copilot Copilot AI left a 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 enforces uniqueness among file-level directives by disallowing duplicates, resolving issue #48842.

  • Added tests in DotnetProjectConvertTests.cs to validate duplicate directive detection.
  • Introduced deduplication logic using a HashSet with a custom NamedDirectiveComparer in VirtualProjectBuildingCommand.cs.
  • Updated localization resources (XLF and RESX files) to include error messages for duplicate directives.

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.

File Description
test/dotnet.Tests/CommandTests/Project/Convert/DotnetProjectConvertTests.cs Added tests to confirm duplicate file-level directive errors are raised correctly.
src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs Implemented deduplication logic by comparing directive type and name.
src/Cli/dotnet/Commands/xlf/* Added DuplicateDirective entries to localization files.
src/Cli/dotnet/Commands/CliCommandStrings.resx Introduced the DuplicateDirective resource entry.

@jaredpar jaredpar added this to the 10.0.1xx milestone Jun 10, 2025
@RikkiGibson RikkiGibson self-assigned this Jun 10, 2025
@RikkiGibson
Copy link
Member

seeing similar CI failures here as in #48387, is there some known issue preventing those dotnet watch tests from succeeding across the board?

@jjonescz
Copy link
Member Author

seeing similar CI failures here as in #48387, is there some known issue preventing those dotnet watch tests from succeeding across the board?

Yes, see #49307.

@jjonescz
Copy link
Member Author

@MiYanni for a review, thanks

@marcpopMSFT
Copy link
Member

Failures are in watch tests which are known, containers test which are known, and a completion test which is known. Merging per request

@marcpopMSFT marcpopMSFT merged commit 69a3f7d into dotnet:main Jun 11, 2025
19 of 30 checks passed
@jjonescz jjonescz deleted the sprint-duplicate-directives branch June 12, 2025 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-run-file Items related to the "dotnet run <file>" effort
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Block duplicate app directives (#:)
5 participants