Merge branch 'main' of https://github.com/dotnet/sdk into darc-main-d77c9975-6517-44d2-bafd-8a16b64c8d6e

Author: Jason Zhai

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.

documentation/project-docs/snapshot-based-testing.md

@@ -0,0 +1,21 @@
+# Guide to snapshot-based testing in the .NET SDK
+
+Snapshot-based testing is a technique used in the .NET SDK to ensure that the command-line interface (CLI) behaves as expected. This document provides an overview of how snapshot-based testing works, particularly for CLI completions, and how to manage and update snapshots.
+
+## CLI Completions Snapshot Testing
+
+The point of the tests is to keep an eye on the CLI, since it is our user interface. When the CLI changes in a way that impacts completions (new commands, options, defaults) we need to inspect and reconcile the baselines. Most of this happens in the [dotnet.Tests][dotnet.Tests] project, in the [Microsoft.DotNet.Cli.Completions.Tests.DotnetCliSnapshotTests][snapshot-tests] tests.
+
+These tests use [Verify][Verify] to perform snapshot testing - storing the results of a known good 'baseline' as a snapshot and comparing the output of the same action performed with changes in your PR. Verify calls these baselines 'verified' files. When the test computes a new snapshot for comparison, this is called a 'received' file. Verify compares the 'verified' file to the 'received' file for each test, and if they are not the same provides a git-diff in the console output for the test.
+
+To fix these tests, you need to diff the two files and visually inspect the changes. If the changes to the 'received' file are what you want to see (new commands, new options, renames, etc) then you rename the 'received' file to 'verified' and commit that change to the 'verified' file. There are two MSBuild Targets on the dotnet.Tests project that you can use to help you do this, both of which are intended to be run after you run the snapshot tests locally:
+
+* [CompareCliSnapshots][compare] - this Target copies the .received. files from the artifacts directory, where they are created due to the way we run tests, to the [snapshots][snapshots] directory in the dotnet.Tests project. This makes it much easier to diff the two.
+* [UpdateCliSnapshots][update] - this Target renames the .received. files to .verified. in the local [snapshots][snapshots] directory, and so acts as a giant 'I accept these changes' button. Only use this if you've diffed the snapshots and are sure they match your expectations.
+
+[dotnet.Tests]: ../../test/dotnet.Tests/
+[snapshot-tests]: ../../test/dotnet.Tests/CompletionTests/DotnetCliSnapshotTests.cs
+[snapshots]: ../../test/dotnet.Tests/CompletionTests/snapshots/
+[Verify]: https://github.com/VerifyTests/Verify
+[compare]: ../../test/dotnet.Tests/dotnet.Tests.csproj#L100
+[update]: ../../test/dotnet.Tests/dotnet.Tests.csproj#L107

src/Cli/Microsoft.DotNet.Cli.Utils/BuiltInCommand.cs

@@ -181,4 +181,9 @@ public ICommand SetCommandArgs(string commandArgs)
     {
         throw new NotImplementedException();
     }
+
+    public ICommand StandardOutputEncoding(Encoding encoding)
+    {
+        throw new NotImplementedException();
+    }
 }

src/Cli/Microsoft.DotNet.Cli.Utils/Command.cs

@@ -101,6 +101,12 @@ public ICommand EnvironmentVariable(string name, string? value)
         return this;
     }
 
+    public ICommand StandardOutputEncoding(Encoding encoding)
+    {
+        _process.StartInfo.StandardOutputEncoding = encoding;
+        return this;
+    }
+
     public ICommand CaptureStdOut()
     {
         ThrowIfRunning();

src/Cli/Microsoft.DotNet.Cli.Utils/ICommand.cs

@@ -25,6 +25,8 @@ public interface ICommand
 
     ICommand SetCommandArgs(string commandArgs);
 
+    ICommand StandardOutputEncoding(Encoding encoding);
+
     string CommandName { get; }
 
     string CommandArgs { get; }

src/Cli/dotnet/CliStrings.resx

@@ -577,8 +577,8 @@ The default is 'false.' However, when targeting .NET 7 or lower, the default is
     <value>outputpathresolver: {0} does not exist</value>
   </data>
   <data name="CannotFindAManifestFile" xml:space="preserve">
-    <value>Cannot find a manifest file.
-For a list of locations searched, specify the "-d" option before the tool name.</value>
+    <value>Cannot find a manifest file. The list of searched paths:
+{0}</value>
   </data>
   <data name="CannotFindPackageIdInManifest" xml:space="preserve">
     <value>Cannot find a package with the package id {0} in the manifest file.</value>
@@ -670,10 +670,6 @@ For a list of locations searched, specify the "-d" option before the tool name.<
   <data name="LibraryNotFoundInLockFile" xml:space="preserve">
     <value>{0}: library not found in lock file.</value>
   </data>
-  <data name="ListOfSearched" xml:space="preserve">
-    <value>The list of searched paths:
-{0}</value>
-  </data>
   <data name="LookingForPreferCliRuntimeFile" xml:space="preserve">
     <value>{0}: Looking for prefercliruntime file at `{1}`</value>
   </data>

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>
@@ -1975,10 +1979,6 @@ Tool '{1}' (version '{2}') was successfully installed.</value>
   <data name="ToolInstallManifestPathOptionName" xml:space="preserve">
     <value>PATH</value>
   </data>
-  <data name="ToolInstallNoManifestGuide" xml:space="preserve">
-    <value>If you intended to install a global tool, add `--global` to the command.
-If you would like to create a manifest, use the `--create-manifest-if-needed` flag with the `dotnet tool install` command, or use `dotnet new tool-manifest`, usually in the repo root directory.</value>
-  </data>
   <data name="ToolInstallNuGetConfigurationFileDoesNotExist" xml:space="preserve">
     <value>NuGet configuration file '{0}' does not exist.</value>
   </data>

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

@@ -110,14 +110,14 @@ public int Execute()
         }
         else
         {
-            if (EntryPointFileFullPath is not null)
+            if (NoCache)
             {
-                projectFactory = CreateVirtualCommand().PrepareProjectInstance().CreateProjectInstance;
+                throw new GracefulException(CliCommandStrings.InvalidOptionCombination, RunCommandParser.NoCacheOption.Name, RunCommandParser.NoBuildOption.Name);
             }
 
-            if (NoCache)
+            if (EntryPointFileFullPath is not null)
             {
-                throw new GracefulException(CliCommandStrings.InvalidOptionCombination, RunCommandParser.NoCacheOption.Name, RunCommandParser.NoBuildOption.Name);
+                projectFactory = CreateVirtualCommand().PrepareProjectInstance().CreateProjectInstance;
             }
         }
 

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

@@ -87,7 +87,6 @@ public VirtualProjectBuildingCommand(
     public override int Execute()
     {
         Debug.Assert(!(NoRestore && NoBuild));
-
         var consoleLogger = RunCommand.MakeTerminalLogger(Verbosity);
         var binaryLogger = GetBinaryLogger(BinaryLoggerArgs);
 
@@ -97,11 +96,6 @@ public override int Execute()
         {
             if (NoCache)
             {
-                if (NoRestore)
-                {
-                    throw new GracefulException(CliCommandStrings.InvalidOptionCombination, RunCommandParser.NoCacheOption.Name, RunCommandParser.NoRestoreOption.Name);
-                }
-
                 cacheEntry = ComputeCacheEntry(out _);
             }
             else if (!NeedsToBuild(out cacheEntry))
@@ -625,6 +619,15 @@ public static void WriteProjectFile(
 
                 """);
 
+            var targetDirectory = Path.GetDirectoryName(targetFilePath) ?? "";
+            writer.WriteLine($"""
+                  <ItemGroup>
+                    <RuntimeHostConfigurationOption Include="EntryPointFilePath" Value="{EscapeValue(targetFilePath)}" />
+                    <RuntimeHostConfigurationOption Include="EntryPointFileDirectoryPath" Value="{EscapeValue(targetDirectory)}" />
+                  </ItemGroup>
+
+                """);
+
             foreach (var sdk in sdkDirectives)
             {
                 WriteImport(writer, "Sdk.targets", sdk);
@@ -709,6 +712,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")]));
@@ -750,6 +754,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);
                 }
             }
@@ -872,7 +898,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
 {
@@ -883,14 +910,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))),
         };
     }
 
@@ -933,14 +960,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)
@@ -967,11 +998,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)
@@ -1007,13 +1037,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)
@@ -1033,6 +1062,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; }

src/Cli/dotnet/Commands/Tool/Install/ToolInstallCommandParser.cs

@@ -49,7 +49,8 @@ internal static class ToolInstallCommandParser
     public static readonly Option<bool> CreateManifestIfNeededOption = new("--create-manifest-if-needed")
     {
         Description = CliCommandStrings.CreateManifestIfNeededOptionDescription,
-        Arity = ArgumentArity.Zero
+        Arity = ArgumentArity.ZeroOrOne,
+        DefaultValueFactory = _ => true,
     };
 
     public static readonly Option<bool> AllowPackageDowngradeOption = new("--allow-downgrade")