File-based apps: ensure shebang analyzer only applies to #:include#54397
File-based apps: ensure shebang analyzer only applies to #:include#54397jjonescz wants to merge 2 commits into
#:include#54397Conversation
There was a problem hiding this comment.
Pull request overview
This PR narrows the CA2266 (“missing shebang in file-based program”) behavior so it only triggers when additional C# source files are brought in via #:include, avoiding warnings for multi-file compilations caused by other MSBuild mechanisms (e.g., Directory.Build.props, project references, etc.).
Changes:
- Emit compiler-visible item metadata in the generated virtual project so analyzers can distinguish
#:include-addedCompileitems. - Update the CA2266 analyzer logic to only warn when a non-entrypoint syntax tree is marked as coming from
#:include. - Adjust/extend CLI and analyzer unit tests to validate the new behavior and the generated project content.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/dotnet.Tests/CommandTests/Run/RunFileTests_Directives.cs | Updates API-based virtual project assertions to require the new compiler-visible metadata declaration. |
| test/dotnet.Tests/CommandTests/Run/RunFileTests_CscOnlyAndApi.cs | Updates expected virtual project XML in CscOnly/API tests to include the compiler-visible metadata item. |
| test/dotnet.Tests/CommandTests/Run/RunFileTests_BuildOptions.cs | Updates CA2266 expectations to ensure no warning from extra Compile items added via Directory.Build.props, while still warning for real #:include. |
| src/Microsoft.DotNet.ProjectTools/VirtualProjectBuilder.cs | Adds <CompilerVisibleItemMetadata ...> so FileBasedProgramsFromIncludeDirective flows into analyzer config as build_metadata.Compile.*. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/tests/.../MissingShebangInFileBasedProgramTests.cs | Updates/extends analyzer tests to cover “include vs non-include extra file” behavior. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/.../CSharpMissingShebangInFileBasedProgram.cs | Changes CA2266 triggering condition to require presence of build_metadata.Compile.FileBasedProgramsFromIncludeDirective=true on at least one non-entrypoint tree. |
|
@RikkiGibson @333fred for reviews, thanks |
|
@RikkiGibson for another review, thanks |
| }); | ||
| } | ||
|
|
||
| private static bool IsFromIncludeDirective(SyntaxTree tree, AnalyzerConfigOptionsProvider optionsProvider) |
There was a problem hiding this comment.
Since we now only want to detect when an #:include happens to be the mechanism including the other file, did you consider making this a syntactic check instead? i.e. just look for an ignored directive whose content starts with include ?
There was a problem hiding this comment.
Perhaps we want to preserve the ability to #:include Config/* and similar, which won't expand into anything that includes .cs, and not tell user to add #! in that case? If so, is there a test for that scenario? I didn't see one from a brief search.
To avoid breaking people that include
.csfiles via other means. See #53749 (comment).