Warn for missing shebang#53614
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new analyzer warning (CA2266) and code fix to help IDEs reliably identify the entry point in multi-file file-based programs by requiring a #! shebang on the entry-point file, and wires up the SDK to provide the entry point file path to analyzers.
Changes:
- Add CA2266 analyzer + code fix for missing shebang in multi-file file-based programs (based on
EntryPointFilePath). - Plumb
EntryPointFilePaththrough the file-based virtual project and generated editorconfig so analyzers can identify the entry point. - Add SDK/integration tests, analyzer unit tests, docs, and resource/localization updates for the new rule.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/dotnet.Tests/CommandTests/Run/RunFileTests.cs | Adds an end-to-end dotnet run Program.cs test validating CA2266 behavior (only for #:include entry points missing a shebang) and updates expected virtual project content. |
| src/Microsoft.DotNet.ProjectTools/VirtualProjectBuilder.cs | Adds <EntryPointFilePath> to the generated virtual project so the entry point path can be surfaced to analyzers. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Usage/MissingShebangInFileBasedProgramTests.cs | Adds analyzer/codefix unit tests for CA2266. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Utilities/Compiler/Options/MSBuildPropertyOptionNames.cs | Adds EntryPointFilePath to the known MSBuild property option names. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt | Extends the Usage range to include CA2266. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hant.xlf | Adds new localized resource entries for CA2266 strings (pending translation). |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hans.xlf | Adds new localized resource entries for CA2266 strings (pending translation). |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.tr.xlf | Adds new localized resource entries for CA2266 strings (pending translation). |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ru.xlf | Adds new localized resource entries for CA2266 strings (pending translation). |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pt-BR.xlf | Adds new localized resource entries for CA2266 strings (pending translation). |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pl.xlf | Adds new localized resource entries for CA2266 strings (pending translation). |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ko.xlf | Adds new localized resource entries for CA2266 strings (pending translation). |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ja.xlf | Adds new localized resource entries for CA2266 strings (pending translation). |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.it.xlf | Adds new localized resource entries for CA2266 strings (pending translation). |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.fr.xlf | Adds new localized resource entries for CA2266 strings (pending translation). |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.es.xlf | Adds new localized resource entries for CA2266 strings (pending translation). |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.de.xlf | Adds new localized resource entries for CA2266 strings (pending translation). |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.cs.xlf | Adds new localized resource entries for CA2266 strings (pending translation). |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/Usage/MissingShebangInFileBasedProgram.cs | Introduces the shared diagnostic descriptor for CA2266. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/Usage/MissingShebangInFileBasedProgram.Fixer.cs | Adds the shared abstract code fix provider base for CA2266. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx | Adds CA2266 title/message/description and code fix title strings. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/AnalyzerReleases.Unshipped.md | Registers CA2266 as a new unshipped rule. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers.sarif | Adds CA2266 metadata to the SARIF rule set. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers.md | Adds documentation entry for CA2266. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.CSharp.NetAnalyzers/Microsoft.NetCore.Analyzers/Usage/CSharpMissingShebangInFileBasedProgram.cs | Implements the C# analyzer logic for CA2266 (checks entry point tree for shebang; reports only when multiple non-generated trees exist). |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.CSharp.NetAnalyzers/Microsoft.NetCore.Analyzers/Usage/CSharpMissingShebangInFileBasedProgram.Fixer.cs | Implements the C# code fix to prepend #!/usr/bin/env dotnet with appropriate EOL. |
| src/Cli/dotnet/Commands/Run/CSharpCompilerCommand.Generated.cs | Exposes EntryPointFilePath to analyzers via generated editorconfig build_property. |
| documentation/general/dotnet-run-file.md | Documents the new EntryPointFilePath exposure and the CA2266 behavior for multi-file file-based programs. |
...alyzers.UnitTests/Microsoft.NetCore.Analyzers/Usage/MissingShebangInFileBasedProgramTests.cs
Show resolved
Hide resolved
| Interlocked.CompareExchange(ref pendingDiagnostic, location.CreateDiagnostic(Rule), null); | ||
| }); | ||
|
|
||
| context.RegisterCompilationEndAction(context => |
There was a problem hiding this comment.
CompilationEnd we don't run in the IDE as live diagnostics; I'm not sure if that's problematic for your scenarios or not. This might be rewritable to some code that would just look at the initial compilation and find the tree that matches the entry point file path, and then you could move the reporting of the diagnostic to when nonGeneratedTreeCount == 2. Somebody else might have a better idea here too...
There was a problem hiding this comment.
Thanks, I haven't realized CompilationEnd doesn't run in IDE. Changed.
| string? directoryPath = AppContext.GetData("EntryPointFileDirectoryPath") as string; | ||
| ``` | ||
|
|
||
| - `EntryPointFilePath` property is set to the entry-point file path and is made visible to analyzers via `CompilerVisibleProperty`. |
There was a problem hiding this comment.
There's a part of me that really feels like this should be something more like a CompilationOption rather than just being passed along as an MSBuild property but I'm not really sure what we'd actually gain with that.
There was a problem hiding this comment.
Well, we have the feature flag FileBasedProgram (which causes roslyn to allow #: and #! directives). We could pass the entry point file path as its value I guess. Feature flags are part of parse options. But I also don't know what we'd gain with that.
There was a problem hiding this comment.
csc has the -main switch today, but, it takes a type name, not a file path..
-main:<type> Specify the type that contains the entry point
(ignore all other possible entry points) (Short
form: -m)
I think that using a compiler-visible msbuild property is the right notch on the dial for the time being.
| } | ||
| } | ||
|
|
||
| return Environment.NewLine; |
There was a problem hiding this comment.
We should have a better answer for this, since there's still .editorconfig settings and the editor to use here as a hint. Looks like for the file header fix we do this:
https://github.com/dotnet/roslyn/blob/106890564f33f6642b625e99bed73b3e9a15244e/src/Analyzers/Core/CodeFixes/FileHeaders/AbstractFileHeaderCodeFixProvider.cs#L48-L50
@JoeRobich or @akhera99 any ideas here?
There was a problem hiding this comment.
I want to say you can annotate the node and it will be formatted when applied.
There was a problem hiding this comment.
There was a problem hiding this comment.
I want to say you can annotate the node and it will be formatted when applied.
It doesn't look like that's happening for newlines inside a trivia like this though.
There was a problem hiding this comment.
Looks like for the file header fix we do this:
Updated to do a similar thing here.
| var eol = GetEndOfLine(root.SyntaxTree.GetText()); | ||
| var shebangTrivia = SyntaxFactory.ParseLeadingTrivia("#!/usr/bin/env dotnet" + eol); | ||
| var firstToken = root.GetFirstToken(includeZeroWidth: true); | ||
| var newFirstToken = firstToken.WithLeadingTrivia(shebangTrivia.AddRange(firstToken.LeadingTrivia)); |
There was a problem hiding this comment.
Where should this go relative to copyright headers?
There was a problem hiding this comment.
#! always needs to be first otherwise it wouldn't be recognized by shells. I remember we already discussed this and the copyright header analyzers should be already handling #! correctly.
| { | ||
| Interlocked.Increment(ref nonGeneratedTreeCount); | ||
|
|
||
| if (!context.Tree.FilePath.Equals(entryPointFilePath, StringComparison.OrdinalIgnoreCase)) |
There was a problem hiding this comment.
The "IgnoreCase" here may be problematic on Linux/Mac, since file names are case sensitive there.
There was a problem hiding this comment.
I guess the file names should be consistent since they come from the same tooling so we can just use Ordinal everywhere.
| /// the <c>generated_code</c> analyzer config option, | ||
| /// common file name patterns, and <c><auto-generated></c> comment headers. | ||
| /// </summary> | ||
| private static bool IsGeneratedCode(SyntaxTree tree, AnalyzerConfigOptionsProvider optionsProvider) |
There was a problem hiding this comment.
I don't like hardcoding this knowledge here, but don't know how to better do this.
Resolves dotnet/roslyn#82944.