Simplify file-level directive diagnostics (#50636)

Author: jjonescz

src/Cli/dotnet/Commands/CliCommandStrings.resx

@@ -1098,12 +1098,12 @@ Tool '{1}' (version '{2}') was successfully installed. Entry is added to the man
     <comment>{0}, {1} number of tests</comment>
   </data>
   <data name="InvalidProjectDirective" xml:space="preserve">
-    <value>The '#:project' directive at '{0}' is invalid: {1}</value>
-    <comment>{0} is the file path and line number. {1} is the inner error message.</comment>
+    <value>The '#:project' directive is invalid: {0}</value>
+    <comment>{0} is the inner error message.</comment>
   </data>
   <data name="MissingDirectiveName" xml:space="preserve">
-    <value>Missing name of '{0}' at {1}.</value>
-    <comment>{0} is the directive name like 'package' or 'sdk', {1} is the file path and line number.</comment>
+    <value>Missing name of '{0}'.</value>
+    <comment>{0} is the directive name like 'package' or 'sdk'.</comment>
   </data>
   <data name="MsBuildDefinition" xml:space="preserve">
     <value>Run Microsoft Build Engine (MSBuild) commands.</value>
@@ -1592,28 +1592,32 @@ Tool '{1}' (version '{2}') was successfully installed. Entry is added to the man
     <value>Project(s)</value>
   </data>
   <data name="PropertyDirectiveInvalidName" xml:space="preserve">
-    <value>Invalid property name at {0}. {1}</value>
-    <comment>{0} is the file path and line number. {1} is an inner exception message.</comment>
+    <value>Invalid property name: {0}</value>
+    <comment>{0} is an inner exception message.</comment>
   </data>
   <data name="PropertyDirectiveMissingParts" xml:space="preserve">
-    <value>The property directive needs to have two parts separated by '=' like '#:property PropertyName=PropertyValue': {0}</value>
-    <comment>{0} is the file path and line number.{Locked="#:property"}</comment>
+    <value>The property directive needs to have two parts separated by '=' like '#:property PropertyName=PropertyValue'.</value>
+    <comment>{Locked="#:property"}</comment>
   </data>
   <data name="StaticGraphRestoreNotSupported" xml:space="preserve">
-    <value>Static graph restore is not supported for file-based apps. Remove the '#:property' at {0}.</value>
-    <comment>{0} is the file path and line number.{Locked="#:property"}</comment>
+    <value>Static graph restore is not supported for file-based apps. Remove the '#:property'.</value>
+    <comment>{Locked="#:property"}</comment>
+  </data>
+  <data name="DirectiveError" xml:space="preserve">
+    <value>error</value>
+    <comment>Used when reporting directive errors like "file(location): error: message".</comment>
   </data>
   <data name="InvalidDirectiveName" xml:space="preserve">
-    <value>The directive at '{2}' should contain a name without special characters and an optional value separated by '{1}' like '#:{0} Name{1}Value'.</value>
-    <comment>{0} is the directive type like 'package' or 'sdk'. {1} is the expected separator like '@' or '='. {2} is the file path and line number.</comment>
+    <value>The directive should contain a name without special characters and an optional value separated by '{1}' like '#:{0} Name{1}Value'.</value>
+    <comment>{0} is the directive type like 'package' or 'sdk'. {1} is the expected separator like '@' or '='.</comment>
   </data>
   <data name="CannotConvertDirective" xml:space="preserve">
-    <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>
+    <value>Some directives cannot be converted. Run the file to see all compilation errors. Specify '--force' to convert anyway.</value>
+    <comment>{Locked="--force"}</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>
+    <value>Duplicate directives are not supported: {0}</value>
+    <comment>{0} is the directive type and name.</comment>
   </data>
   <data name="InvalidOptionForStdin" xml:space="preserve">
     <value>Cannot specify option '{0}' when also using '-' to read the file from standard input.</value>
@@ -2252,8 +2256,8 @@ and the corresponding package Ids for installed tools using the command
     <value>Rows with Unlogged changes represent changes from actions other than .NET CLI workload commands. Usually this represents an update to the .NET SDK or to Visual Studio.</value>
   </data>
   <data name="UnrecognizedDirective" xml:space="preserve">
-    <value>Unrecognized directive '{0}' at {1}.</value>
-    <comment>{0} is the directive name like 'package' or 'sdk', {1} is the file path and line number.</comment>
+    <value>Unrecognized directive '{0}'.</value>
+    <comment>{0} is the directive name like 'package' or 'sdk'.</comment>
   </data>
   <data name="UpdateAllOptionDescription" xml:space="preserve">
     <value>Update all tools.</value>

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

@@ -1488,7 +1488,7 @@ public static ImmutableArray<CSharpDirective> FindDirectives(SourceFile sourceFi
                     if (deduplicated.TryGetValue(directive, out var existingDirective))
                     {
                         var typeAndName = $"#:{existingDirective.GetType().Name.ToLowerInvariant()} {existingDirective.Name}";
-                        diagnostics.AddError(sourceFile, directive.Info.Span, location => string.Format(CliCommandStrings.DuplicateDirective, typeAndName, location));
+                        diagnostics.AddError(sourceFile, directive.Info.Span, string.Format(CliCommandStrings.DuplicateDirective, typeAndName));
                     }
                     else
                     {
@@ -1538,7 +1538,7 @@ void ReportErrorFor(SyntaxTrivia trivia)
         {
             if (trivia.ContainsDiagnostics && trivia.IsKind(SyntaxKind.IgnoredDirectiveTrivia))
             {
-                diagnostics.AddError(sourceFile, trivia.Span, location => string.Format(CliCommandStrings.CannotConvertDirective, location));
+                diagnostics.AddError(sourceFile, trivia.Span, CliCommandStrings.CannotConvertDirective);
             }
         }
 
@@ -1660,7 +1660,7 @@ public FileLinePositionSpan GetFileLinePositionSpan(TextSpan span)
     public string GetLocationString(TextSpan span)
     {
         var positionSpan = GetFileLinePositionSpan(span);
-        return $"{positionSpan.Path}:{positionSpan.StartLinePosition.Line + 1}";
+        return $"{positionSpan.Path}({positionSpan.StartLinePosition.Line + 1})";
     }
 }
 
@@ -1717,7 +1717,7 @@ public readonly struct ParseContext
             "property" => Property.Parse(context),
             "package" => Package.Parse(context),
             "project" => Project.Parse(context),
-            var other => context.Diagnostics.AddError<Named>(context.SourceFile, context.Info.Span, location => string.Format(CliCommandStrings.UnrecognizedDirective, other, location)),
+            var other => context.Diagnostics.AddError<Named>(context.SourceFile, context.Info.Span, string.Format(CliCommandStrings.UnrecognizedDirective, other)),
         };
     }
 
@@ -1729,13 +1729,13 @@ private static (string, string?)? ParseOptionalTwoParts(in ParseContext context,
         string directiveKind = context.DirectiveKind;
         if (firstPart.IsWhiteSpace())
         {
-            return context.Diagnostics.AddError<(string, string?)?>(context.SourceFile, context.Info.Span, location => string.Format(CliCommandStrings.MissingDirectiveName, directiveKind, location));
+            return context.Diagnostics.AddError<(string, string?)?>(context.SourceFile, context.Info.Span, string.Format(CliCommandStrings.MissingDirectiveName, directiveKind));
         }
 
         // If the name contains characters that resemble separators, report an error to avoid any confusion.
         if (Patterns.DisallowedNameCharacters.IsMatch(firstPart))
         {
-            return context.Diagnostics.AddError<(string, string?)?>(context.SourceFile, context.Info.Span, location => string.Format(CliCommandStrings.InvalidDirectiveName, directiveKind, separator, location));
+            return context.Diagnostics.AddError<(string, string?)?>(context.SourceFile, context.Info.Span, string.Format(CliCommandStrings.InvalidDirectiveName, directiveKind, separator));
         }
 
         var secondPart = i < 0 ? [] : context.DirectiveText.AsSpan((i + 1)..).TrimStart();
@@ -1802,7 +1802,7 @@ public sealed class Property(in ParseInfo info) : Named(info)
 
             if (propertyValue is null)
             {
-                return context.Diagnostics.AddError<Property?>(context.SourceFile, context.Info.Span, static location => string.Format(CliCommandStrings.PropertyDirectiveMissingParts, location));
+                return context.Diagnostics.AddError<Property?>(context.SourceFile, context.Info.Span, CliCommandStrings.PropertyDirectiveMissingParts);
             }
 
             try
@@ -1811,13 +1811,13 @@ public sealed class Property(in ParseInfo info) : Named(info)
             }
             catch (XmlException ex)
             {
-                return context.Diagnostics.AddError<Property?>(context.SourceFile, context.Info.Span, location => string.Format(CliCommandStrings.PropertyDirectiveInvalidName, location, ex.Message), ex);
+                return context.Diagnostics.AddError<Property?>(context.SourceFile, context.Info.Span, string.Format(CliCommandStrings.PropertyDirectiveInvalidName, ex.Message), ex);
             }
 
             if (propertyName.Equals("RestoreUseStaticGraphEvaluation", StringComparison.OrdinalIgnoreCase) &&
                 MSBuildUtilities.ConvertStringToBool(propertyValue))
             {
-                context.Diagnostics.AddError(context.SourceFile, context.Info.Span, static location => string.Format(CliCommandStrings.StaticGraphRestoreNotSupported, location));
+                context.Diagnostics.AddError(context.SourceFile, context.Info.Span, CliCommandStrings.StaticGraphRestoreNotSupported);
             }
 
             return new Property(context.Info)
@@ -1865,7 +1865,7 @@ public sealed class Project(in ParseInfo info) : Named(info)
             if (directiveText.IsWhiteSpace())
             {
                 string directiveKind = context.DirectiveKind;
-                return context.Diagnostics.AddError<Project?>(context.SourceFile, context.Info.Span, location => string.Format(CliCommandStrings.MissingDirectiveName, directiveKind, location));
+                return context.Diagnostics.AddError<Project?>(context.SourceFile, context.Info.Span, string.Format(CliCommandStrings.MissingDirectiveName, directiveKind));
             }
 
             try
@@ -1886,7 +1886,7 @@ public sealed class Project(in ParseInfo info) : Named(info)
             }
             catch (GracefulException e)
             {
-                context.Diagnostics.AddError(context.SourceFile, context.Info.Span, location => string.Format(CliCommandStrings.InvalidProjectDirective, location, e.Message), e);
+                context.Diagnostics.AddError(context.SourceFile, context.Info.Span, string.Format(CliCommandStrings.InvalidProjectDirective, e.Message), e);
             }
 
             return new Project(context.Info)
@@ -1960,22 +1960,22 @@ internal readonly struct DiagnosticBag
     public static DiagnosticBag Collect(out ImmutableArray<SimpleDiagnostic>.Builder builder) => new() { Builder = builder = ImmutableArray.CreateBuilder<SimpleDiagnostic>() };
     public static DiagnosticBag Ignore() => new() { IgnoreDiagnostics = true, Builder = null };
 
-    public void AddError(SourceFile sourceFile, TextSpan span, Func<string, string> messageFactory, Exception? inner = null)
+    public void AddError(SourceFile sourceFile, TextSpan span, string message, Exception? inner = null)
     {
         if (Builder != null)
         {
             Debug.Assert(!IgnoreDiagnostics);
-            Builder.Add(new SimpleDiagnostic { Location = sourceFile.GetFileLinePositionSpan(span), Message = messageFactory(sourceFile.GetLocationString(span)) });
+            Builder.Add(new SimpleDiagnostic { Location = sourceFile.GetFileLinePositionSpan(span), Message = message });
         }
         else if (!IgnoreDiagnostics)
         {
-            throw new GracefulException(messageFactory(sourceFile.GetLocationString(span)), inner);
+            throw new GracefulException($"{sourceFile.GetLocationString(span)}: {CliCommandStrings.DirectiveError}: {message}", inner);
         }
     }
 
-    public T? AddError<T>(SourceFile sourceFile, TextSpan span, Func<string, string> messageFactory, Exception? inner = null)
+    public T? AddError<T>(SourceFile sourceFile, TextSpan span, string message, Exception? inner = null)
     {
-        AddError(sourceFile, span, messageFactory, inner);
+        AddError(sourceFile, span, message, inner);
         return default;
     }
 }