Disallow quotes in file-level directives#51119
Disallow quotes in file-level directives#51119jjonescz merged 3 commits intodotnet:release/10.0.1xxfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces validation to disallow quotes in file-level directives and surfaces a new localized error message when quotes are present.
- Adds diagnostic reporting when a directive value contains a double quote.
- Updates tests to assert new diagnostics instead of previous property name validation in affected cases.
- Introduces a new localized resource (QuoteInDirective) across all .xlf language files.
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/dotnet.Tests/CommandTests/Project/Convert/DotnetProjectConvertTests.cs | Adds expected directive diagnostics for quotes, extends helper methods to collect and verify diagnostics, introduces constant for program path. |
| src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs | Adds validation rejecting directives containing quotes and emits new diagnostic. |
| src/Cli/dotnet/Commands/CliCommandStrings.resx | Adds new resource string for quote restriction error. |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.de.xlf | Adds localization entry for new error (manually edited). |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.es.xlf | Adds localization entry for new error (manually edited). |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.fr.xlf | Adds localization entry for new error (manually edited). |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.it.xlf | Adds localization entry for new error (manually edited). |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.ja.xlf | Adds localization entry for new error (manually edited). |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.ko.xlf | Adds localization entry for new error (manually edited). |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.pl.xlf | Adds localization entry for new error (manually edited). |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.pt-BR.xlf | Adds localization entry for new error (manually edited). |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.ru.xlf | Adds localization entry for new error (manually edited). |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.tr.xlf | Adds localization entry for new error (manually edited). |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.cs.xlf | Adds localization entry for new error (manually edited). |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.zh-Hans.xlf | Adds localization entry for new error (manually edited). |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.zh-Hant.xlf | Adds localization entry for new error (manually edited). |
test/dotnet.Tests/CommandTests/Project/Convert/DotnetProjectConvertTests.cs
Show resolved
Hide resolved
|
@RikkiGibson @333fred @MiYanni for reviews, thanks |
|
taking a look shortly. |
RikkiGibson
left a comment
There was a problem hiding this comment.
It would be good to also demonstrate what happens when user tries to escape the quote, e.g. #:property Name=\"Value\".
RikkiGibson
left a comment
There was a problem hiding this comment.
LGTM aside from a test suggestion.
| <comment>{0} is the directive type and name.</comment> | ||
| </data> | ||
| <data name="QuoteInDirective" xml:space="preserve"> | ||
| <value>Directives currently cannot contain quotes (").</value> |
There was a problem hiding this comment.
So, is this specific to double quotes " or does that also account for single quotes '? Might make sense to be a bit more specific.
There was a problem hiding this comment.
Currently this is specific to double quotes, I will clarify, thanks. We could disallow single quotes too, I didn't think about that, but it also seems unnecessary (we likely won't want to support single-quoted directive values anyway, only double-quoted, I imagine, same as C# strings).
|
Added When you commit this breaking change:
You can refer to the .NET SDK breaking change guidelines |
Description
This blocks usage of quotes
"inside#:file-level directives, so we can later support quoted directives (#49367) without a breaking change. This also improves the error recovery experience if users try to use quotes now thinking that's a supported syntax.Customer impact
If customers have quotes in their file-level directives (which seems unlikely, there is no known use case for that), they will now see errors (and would need to move these directives outside the file-based app into a Directory.Build.props file for example until we support quoted directives - this is similar to other special characters we currently don't support in unquoted directives, like trailing whitespace).
Regression
N/A.
Risk
Low, changing a new feature in .NET 10, disallowing an edge case that is unlikely to be relied upon. Will avoid a future breaking change.
Breaking change doc: dotnet/docs#48916