Disallow duplicate file-level directives (#49308)

Author: marcpopMSFT

documentation/general/dotnet-run-file.md

@@ -71,11 +71,11 @@ We want to report an error for non-entry-point files to avoid the confusion of b
 
 Internally, the SDK CLI detects entry points by parsing all `.cs` files in the directory tree of the entry point file with default parsing options (in particular, no `<DefineConstants>`)
 and checking which ones contain top-level statements (`Main` methods are not supported for now as that would require full semantic analysis, not just parsing).
-Results of this detection are used to exclude other entry points from [builds](#multiple-entry-points) and [app directive collection](#directives-for-project-metadata).
+Results of this detection are used to exclude other entry points from [builds](#multiple-entry-points) and [file-level directive collection](#directives-for-project-metadata).
 This means the CLI might consider a file to be an entry point which later the compiler doesn't
 (for example because its top-level statements are under `#if !SYMBOL` and the build has `DefineConstants=SYMBOL`).
 However such inconsistencies should be rare and hence that is a better trade off than letting the compiler decide which files are entry points
-because that could require multiple builds (first determine entry points and then re-build with app directives except those from other entry points).
+because that could require multiple builds (first determine entry points and then re-build with file-level directives except those from other entry points).
 To avoid parsing all C# files twice (in CLI and in the compiler), the CLI could use the compiler server for parsing so the trees are reused
 (unless the parse options change via the directives), and also [cache](#optimizations) the results to avoid parsing on subsequent runs.
 
@@ -146,7 +146,7 @@ They are not cleaned immediately because they can be re-used on subsequent runs
 
 ## Directives for project metadata
 
-It is possible to specify some project metadata via *app directives*
+It is possible to specify some project metadata via *file-level directives*
 which are [ignored][ignored-directives] by the C# language but recognized by the SDK CLI.
 Directives `sdk`, `package`, and `property` are translated into `<Project Sdk="...">`, `<PackageReference>`, and `<Property>` project elements, respectively.
 Other directives result in an error, reserving them for future use.

src/Cli/dotnet/Commands/CliCommandStrings.resx

@@ -1513,6 +1513,10 @@ Tool '{1}' (version '{2}') was successfully installed. Entry is added to the man
     <value>Some directives cannot be converted: the first error is at {0}. Run the file to see all compilation errors. Specify '--force' to convert anyway.</value>
     <comment>{Locked="--force"}. {0} is the file path and line number.</comment>
   </data>
+  <data name="DuplicateDirective" xml:space="preserve">
+    <value>Duplicate directives are not supported: {0} at {1}</value>
+    <comment>{0} is the directive type and name. {1} is the file path and line number.</comment>
+  </data>
   <data name="InvalidOptionCombination" xml:space="preserve">
     <value>Cannot combine option '{0}' and '{1}'.</value>
     <comment>{0} and {1} are option names like '--no-build'.</comment>

src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs

@@ -704,6 +704,7 @@ public static ImmutableArray<CSharpDirective> FindDirectives(SourceFile sourceFi
     {
 #pragma warning disable RSEXPERIMENTAL003 // 'SyntaxTokenParser' is experimental
 
+        var deduplicated = new HashSet<CSharpDirective.Named>(NamedDirectiveComparer.Instance);
         var builder = ImmutableArray.CreateBuilder<CSharpDirective>();
         SyntaxTokenParser tokenizer = SyntaxFactory.CreateTokenParser(sourceFile.Text,
             CSharpParseOptions.Default.WithFeatures([new("FileBasedProgram", "true")]));
@@ -745,6 +746,28 @@ public static ImmutableArray<CSharpDirective> FindDirectives(SourceFile sourceFi
 
                 if (CSharpDirective.Parse(errors, sourceFile, span, name.ToString(), value.ToString()) is { } directive)
                 {
+                    // If the directive is already present, report an error.
+                    if (deduplicated.TryGetValue(directive, out var existingDirective))
+                    {
+                        var typeAndName = $"#:{existingDirective.GetType().Name.ToLowerInvariant()} {existingDirective.Name}";
+                        if (errors != null)
+                        {
+                            errors.Add(new SimpleDiagnostic
+                            {
+                                Location = sourceFile.GetFileLinePositionSpan(directive.Span),
+                                Message = string.Format(CliCommandStrings.DuplicateDirective, typeAndName, sourceFile.GetLocationString(directive.Span)),
+                            });
+                        }
+                        else
+                        {
+                            throw new GracefulException(CliCommandStrings.DuplicateDirective, typeAndName, sourceFile.GetLocationString(directive.Span));
+                        }
+                    }
+                    else
+                    {
+                        deduplicated.Add(directive);
+                    }
+
                     builder.Add(directive);
                 }
             }
@@ -867,7 +890,8 @@ internal static partial class Patterns
 }
 
 /// <summary>
-/// Represents a C# directive starting with <c>#:</c>. Those are ignored by the language but recognized by us.
+/// Represents a C# directive starting with <c>#:</c> (a.k.a., "file-level directive").
+/// Those are ignored by the language but recognized by us.
 /// </summary>
 internal abstract class CSharpDirective
 {
@@ -878,14 +902,14 @@ private CSharpDirective() { }
     /// </summary>
     public required TextSpan Span { get; init; }
 
-    public static CSharpDirective? Parse(ImmutableArray<SimpleDiagnostic>.Builder? errors, SourceFile sourceFile, TextSpan span, string directiveKind, string directiveText)
+    public static Named? Parse(ImmutableArray<SimpleDiagnostic>.Builder? errors, SourceFile sourceFile, TextSpan span, string directiveKind, string directiveText)
     {
         return directiveKind switch
         {
             "sdk" => Sdk.Parse(errors, sourceFile, span, directiveKind, directiveText),
             "property" => Property.Parse(errors, sourceFile, span, directiveKind, directiveText),
             "package" => Package.Parse(errors, sourceFile, span, directiveKind, directiveText),
-            _ => ReportError<CSharpDirective>(errors, sourceFile, span, string.Format(CliCommandStrings.UnrecognizedDirective, directiveKind, sourceFile.GetLocationString(span))),
+            _ => ReportError<Named>(errors, sourceFile, span, string.Format(CliCommandStrings.UnrecognizedDirective, directiveKind, sourceFile.GetLocationString(span))),
         };
     }
 
@@ -928,14 +952,18 @@ private static (string, string?)? ParseOptionalTwoParts(ImmutableArray<SimpleDia
     /// </summary>
     public sealed class Shebang : CSharpDirective;
 
+    public abstract class Named : CSharpDirective
+    {
+        public required string Name { get; init; }
+    }
+
     /// <summary>
     /// <c>#:sdk</c> directive.
     /// </summary>
-    public sealed class Sdk : CSharpDirective
+    public sealed class Sdk : Named
     {
         private Sdk() { }
 
-        public required string Name { get; init; }
         public string? Version { get; init; }
 
         public static new Sdk? Parse(ImmutableArray<SimpleDiagnostic>.Builder? errors, SourceFile sourceFile, TextSpan span, string directiveKind, string directiveText)
@@ -962,11 +990,10 @@ public string ToSlashDelimitedString()
     /// <summary>
     /// <c>#:property</c> directive.
     /// </summary>
-    public sealed class Property : CSharpDirective
+    public sealed class Property : Named
     {
         private Property() { }
 
-        public required string Name { get; init; }
         public required string Value { get; init; }
 
         public static new Property? Parse(ImmutableArray<SimpleDiagnostic>.Builder? errors, SourceFile sourceFile, TextSpan span, string directiveKind, string directiveText)
@@ -1002,13 +1029,12 @@ private Property() { }
     /// <summary>
     /// <c>#:package</c> directive.
     /// </summary>
-    public sealed class Package : CSharpDirective
+    public sealed class Package : Named
     {
         private static readonly SearchValues<char> s_separators = SearchValues.Create(' ', '@');
 
         private Package() { }
 
-        public required string Name { get; init; }
         public string? Version { get; init; }
 
         public static new Package? Parse(ImmutableArray<SimpleDiagnostic>.Builder? errors, SourceFile sourceFile, TextSpan span, string directiveKind, string directiveText)
@@ -1028,6 +1054,33 @@ private Package() { }
     }
 }
 
+/// <summary>
+/// Used for deduplication - compares directives by their type and name (ignoring case).
+/// </summary>
+internal sealed class NamedDirectiveComparer : IEqualityComparer<CSharpDirective.Named>
+{
+    public static readonly NamedDirectiveComparer Instance = new();
+
+    private NamedDirectiveComparer() { }
+
+    public bool Equals(CSharpDirective.Named? x, CSharpDirective.Named? y)
+    {
+        if (ReferenceEquals(x, y)) return true;
+
+        if (x is null || y is null) return false;
+
+        return x.GetType() == y.GetType() &&
+            string.Equals(x.Name, y.Name, StringComparison.OrdinalIgnoreCase);
+    }
+
+    public int GetHashCode(CSharpDirective.Named obj)
+    {
+        return HashCode.Combine(
+            obj.GetType().GetHashCode(),
+            obj.Name.GetHashCode(StringComparison.OrdinalIgnoreCase));
+    }
+}
+
 internal sealed class SimpleDiagnostic
 {
     public required Position Location { get; init; }