Improve service reference filenames and class names (#7447)

- #4927
- fully-sanitize class names and filenames
  - use aspnet/AspNetCore-Tooling's `CSharpIdentifier` class
- default metadata in sequence [URI or project&document name ->] `%(DocumentPath)` -> `%(OutputPath)` -> `%(ClassName)`
  - if user sets metadata explicitly, the override affects defaults later in the sequence
- separate some nested validations and defaulting steps
  - provide default `%(DocumentName}` even if `%(DocumentPath}` is set explicitly
  - validate URI is absolute even if `%(DocumentPath}` is set explicitly

other:
- don't write out an empty Open API / Swagger file

nits:
- do not use default `%(DocumentName}` in default `%(DocumentPath)` for `<ServiceProjectReference>` items
- do not use empty URI path or query string in default `%(DocumentPath)` for `<ServiceUriReference>` items

src/Mvc/Extensions.ApiDescription.Design/src/CSharpIdentifier.cs

@@ -0,0 +1,55 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Globalization;
+using System.Text;
+
+// Copied from
+// https://github.com/aspnet/AspNetCore-Tooling/blob/master/src/Razor/src/Microsoft.AspNetCore.Razor.Language/CSharpIdentifier.cs
+namespace Microsoft.Extensions.ApiDescription.Tasks
+{
+    internal static class CSharpIdentifier
+    {
+        // CSharp Spec §2.4.2
+        private static bool IsIdentifierStart(char character)
+        {
+            return char.IsLetter(character) ||
+                character == '_' ||
+                CharUnicodeInfo.GetUnicodeCategory(character) == UnicodeCategory.LetterNumber;
+        }
+
+        public static bool IsIdentifierPart(char character)
+        {
+            return char.IsDigit(character) ||
+                   IsIdentifierStart(character) ||
+                   IsIdentifierPartByUnicodeCategory(character);
+        }
+
+        private static bool IsIdentifierPartByUnicodeCategory(char character)
+        {
+            var category = CharUnicodeInfo.GetUnicodeCategory(character);
+
+            return category == UnicodeCategory.NonSpacingMark || // Mn
+                category == UnicodeCategory.SpacingCombiningMark || // Mc
+                category == UnicodeCategory.ConnectorPunctuation || // Pc
+                category == UnicodeCategory.Format; // Cf
+        }
+
+        public static string SanitizeIdentifier(string inputName)
+        {
+            if (!IsIdentifierStart(inputName[0]) && IsIdentifierPart(inputName[0]))
+            {
+                inputName = "_" + inputName;
+            }
+
+            var builder = new StringBuilder(inputName.Length);
+            for (var i = 0; i < inputName.Length; i++)
+            {
+                var ch = inputName[i];
+                builder.Append(IsIdentifierPart(ch) ? ch : '_');
+            }
+
+            return builder.ToString();
+        }
+    }
+}

src/Mvc/Extensions.ApiDescription.Design/src/GetFileReferenceMetadata.cs

@@ -78,30 +78,13 @@ namespace Microsoft.Extensions.ApiDescription.Tasks
                     Log.LogError(Resources.FormatInvalidEmptyMetadataValue("CodeGenerator", type, item.ItemSpec));
                 }
 
-                var className = item.GetMetadata("ClassName");
-                if (string.IsNullOrEmpty(className))
-                {
-                    var filename = item.GetMetadata("Filename");
-                    className = $"{filename}Client";
-                    if (char.IsLower(className[0]))
-                    {
-                        className = char.ToUpper(className[0]) + className.Substring(startIndex: 1);
-                    }
-
-                    MetadataSerializer.SetMetadata(newItem, "ClassName", className);
-                }
-
-                var @namespace = item.GetMetadata("Namespace");
-                if (string.IsNullOrEmpty(@namespace))
-                {
-                    MetadataSerializer.SetMetadata(newItem, "Namespace", Namespace);
-                }
-
                 var outputPath = item.GetMetadata("OutputPath");
                 if (string.IsNullOrEmpty(outputPath))
                 {
+                    // No need to further sanitize this path.
+                    var filename = item.GetMetadata("Filename");
                     var isTypeScript = codeGenerator.EndsWith(TypeScriptLanguageName, StringComparison.OrdinalIgnoreCase);
-                    outputPath = $"{className}{(isTypeScript ? ".ts" : Extension)}";
+                    outputPath = $"{filename}Client{(isTypeScript ? ".ts" : Extension)}";
                 }
 
                 // Place output file in correct directory (relative to project directory).
@@ -119,6 +102,21 @@ namespace Microsoft.Extensions.ApiDescription.Tasks
 
                 MetadataSerializer.SetMetadata(newItem, "OutputPath", outputPath);
 
+                var className = item.GetMetadata("ClassName");
+                if (string.IsNullOrEmpty(className))
+                {
+                    var outputFilename = Path.GetFileNameWithoutExtension(outputPath);
+
+                    className = CSharpIdentifier.SanitizeIdentifier(outputFilename);
+                    MetadataSerializer.SetMetadata(newItem, "ClassName", className);
+                }
+
+                var @namespace = item.GetMetadata("Namespace");
+                if (string.IsNullOrEmpty(@namespace))
+                {
+                    MetadataSerializer.SetMetadata(newItem, "Namespace", Namespace);
+                }
+
                 // Add metadata which may be used as a property and passed to an inner build.
                 newItem.RemoveMetadata("SerializedMetadata");
                 newItem.SetMetadata("SerializedMetadata", MetadataSerializer.SerializeMetadata(newItem));

src/Mvc/Extensions.ApiDescription.Design/src/GetProjectReferenceMetadata.cs

@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
 using System.Collections.Generic;
 using System.IO;
 using Microsoft.Build.Framework;
@@ -14,6 +15,9 @@ namespace Microsoft.Extensions.ApiDescription.Tasks
     /// </summary>
     public class GetProjectReferenceMetadata : Task
     {
+        private static readonly char[] InvalidFilenameCharacters = Path.GetInvalidFileNameChars();
+        private static readonly string[] InvalidFilenameStrings = new[] { ".." };
+
         /// <summary>
         /// Default directory for DocumentPath values.
         /// </summary>
@@ -53,22 +57,60 @@ namespace Microsoft.Extensions.ApiDescription.Tasks
                         item.ItemSpec));
                 }
 
+                var documentName = item.GetMetadata("DocumentName");
+                if (string.IsNullOrEmpty(documentName))
+                {
+                    documentName = "v1";
+                    MetadataSerializer.SetMetadata(newItem, "DocumentName", documentName);
+                }
+
                 var documentPath = item.GetMetadata("DocumentPath");
                 if (string.IsNullOrEmpty(documentPath))
                 {
-                    var filename = item.GetMetadata("Filename");
+                    // No need to sanitize the filename since the project file exists.
-                    var documentName = item.GetMetadata("DocumentName");
+                    var projectFilename = item.GetMetadata("Filename");
+
+                    // Default document filename matches project filename unless given a non-default document name.
                     if (string.IsNullOrEmpty(documentName))
                     {
-                        documentName = "v1";
+                        // This is an odd (but allowed) case that would break the sanitize one-liner below. Also,
+                        // ensure chosen name does not match the "v1" case.
+                        documentPath = projectFilename + "_.json";
                     }
+                    else if (string.Equals("v1", documentName, StringComparison.Ordinal))
+                    {
+                        documentPath = projectFilename + ".json";
+                    }
+                    else
+                    {
+                        // Sanitize the document name because it may contain almost any character, including illegal
+                        // filename characters such as '/' and '?'. (Do not treat slashes as folder separators.)
+                        var sanitizedDocumentName = string.Join("_", documentName.Split(InvalidFilenameCharacters));
+                        while (sanitizedDocumentName.Contains(InvalidFilenameStrings[0]))
+                        {
+                            sanitizedDocumentName = string.Join(
+                                ".",
+                                sanitizedDocumentName.Split(InvalidFilenameStrings, StringSplitOptions.None));
+                        }
 
-                    documentPath = $"{filename}.{documentName}.json";
+                        documentPath = $"{projectFilename}_{sanitizedDocumentName}";
+
+                        // Possible the document path already ends with .json. Don't duplicate that or a final period.
+                        if (!documentPath.EndsWith(".json", StringComparison.OrdinalIgnoreCase))
+                        {
+                            if (documentPath.EndsWith(".", StringComparison.Ordinal))
+                            {
+                                documentPath += "json";
+                            }
+                            else
+                            {
+                                documentPath += ".json";
+                            }
+                        }
+                    }
                 }
 
                 documentPath = GetFullPath(documentPath);
-                MetadataSerializer.SetMetadata(newItem, "DocumentPath", documentPath);
-
                 if (!destinations.Add(documentPath))
                 {
                     // This case may occur when user is experimenting e.g. with multiple generators or options.
@@ -76,6 +118,8 @@ namespace Microsoft.Extensions.ApiDescription.Tasks
                     Log.LogError(Resources.FormatDuplicateProjectDocumentPaths(documentPath));
                 }
 
+                MetadataSerializer.SetMetadata(newItem, "DocumentPath", documentPath);
+
                 // Add metadata which may be used as a property and passed to an inner build.
                 newItem.SetMetadata("SerializedMetadata", MetadataSerializer.SerializeMetadata(newItem));
             }

src/Mvc/Extensions.ApiDescription.Design/src/GetUriReferenceMetadata.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Text;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Utilities;
 
@@ -14,6 +15,9 @@ namespace Microsoft.Extensions.ApiDescription.Tasks
     /// </summary>
     public class GetUriReferenceMetadata : Task
     {
+        private static readonly char[] InvalidFilenameCharacters = Path.GetInvalidFileNameChars();
+        private static readonly string[] InvalidFilenameStrings = new[] { ".." };
+
         /// <summary>
         /// Default directory for DocumentPath metadata values.
         /// </summary>
@@ -41,68 +45,74 @@ namespace Microsoft.Extensions.ApiDescription.Tasks
                 var newItem = new TaskItem(item);
                 outputs.Add(newItem);
 
+                var uri = item.ItemSpec;
+                var builder = new UriBuilder(uri);
+                if (!builder.Uri.IsAbsoluteUri)
+                {
+                    Log.LogError($"{nameof(Inputs)} item '{uri}' is not an absolute URI.");
+                    return false;
+                }
+
+                if (!string.Equals(Uri.UriSchemeHttp, builder.Scheme, StringComparison.OrdinalIgnoreCase) &&
+                    !string.Equals(Uri.UriSchemeHttps, builder.Scheme, StringComparison.OrdinalIgnoreCase))
+                {
+                    Log.LogError($"{nameof(Inputs)} item '{uri}' does not have scheme {Uri.UriSchemeHttp} or " +
+                        $"{Uri.UriSchemeHttps}.");
+                    return false;
+                }
+
+                // If not specified, base filename on the URI.
                 var documentPath = item.GetMetadata("DocumentPath");
                 if (string.IsNullOrEmpty(documentPath))
                 {
-                    var uri = item.ItemSpec;
+                    // Default to a fairly long but identifiable and fairly unique filename.
-                    var builder = new UriBuilder(uri);
+                    var documentPathBuilder = new StringBuilder(builder.Host);
-                    if (!builder.Uri.IsAbsoluteUri)
+                    if (!string.IsNullOrEmpty(builder.Path) &&
+                        !string.Equals("/", builder.Path, StringComparison.Ordinal))
                     {
-                        Log.LogError($"{nameof(Inputs)} item '{uri}' is not an absolute URI.");
+                        documentPathBuilder.Append(builder.Path);
-                        return false;
                     }
 
-                    if (!string.Equals(Uri.UriSchemeHttp, builder.Scheme, StringComparison.OrdinalIgnoreCase) &&
+                    if (!string.IsNullOrEmpty(builder.Query) &&
-                        !string.Equals(Uri.UriSchemeHttps, builder.Scheme, StringComparison.OrdinalIgnoreCase))
+                        !string.Equals("?", builder.Query, StringComparison.Ordinal))
                     {
-                        Log.LogError($"{nameof(Inputs)} item '{uri}' does not have scheme {Uri.UriSchemeHttp} or " +
+                        documentPathBuilder.Append(builder.Query);
-                            $"{Uri.UriSchemeHttps}.");
-                        return false;
                     }
 
-                    var host = builder.Host
+                    // Sanitize the string because it likely contains illegal filename characters such as '/' and '?'.
-                      .Replace("/", string.Empty)
+                    // (Do not treat slashes as folder separators.)
-                      .Replace("[", string.Empty)
+                    documentPath = documentPathBuilder.ToString();
-                      .Replace("]", string.Empty)
+                    documentPath = string.Join("_", documentPath.Split(InvalidFilenameCharacters));
-                      .Replace(':', '_');
+                    while (documentPath.Contains(InvalidFilenameStrings[0]))
-                    var path = builder.Path
-                      .Replace("!", string.Empty)
-                      .Replace("'", string.Empty)
-                      .Replace("$", string.Empty)
-                      .Replace("%", string.Empty)
-                      .Replace("&", string.Empty)
-                      .Replace("(", string.Empty)
-                      .Replace(")", string.Empty)
-                      .Replace("*", string.Empty)
-                      .Replace("@", string.Empty)
-                      .Replace("~", string.Empty)
-                      .Replace('/', '_')
-                      .Replace(':', '_')
-                      .Replace(';', '_')
-                      .Replace('+', '_')
-                      .Replace('=', '_');
-
-                    documentPath = host + path;
-                    if (char.IsLower(documentPath[0]))
                     {
-                        documentPath = char.ToUpper(documentPath[0]) + documentPath.Substring(startIndex: 1);
+                        documentPath = string.Join(
+                            ".",
+                            documentPath.Split(InvalidFilenameStrings, StringSplitOptions.None));
                     }
 
+                    // URI might end with ".json". Don't duplicate that or a final period.
                     if (!documentPath.EndsWith(".json", StringComparison.OrdinalIgnoreCase))
                     {
-                        documentPath = $"{documentPath}.json";
+                        if (documentPath.EndsWith(".", StringComparison.Ordinal))
+                        {
+                            documentPath += "json";
+                        }
+                        else
+                        {
+                            documentPath += ".json";
+                        }
                     }
                 }
 
                 documentPath = GetFullPath(documentPath);
-                MetadataSerializer.SetMetadata(newItem, "DocumentPath", documentPath);
-
                 if (!destinations.Add(documentPath))
                 {
                     // This case may occur when user is experimenting e.g. with multiple code generators or options.
                     // May also occur when user accidentally duplicates DocumentPath metadata.
                     Log.LogError(Resources.FormatDuplicateUriDocumentPaths(documentPath));
                 }
+
+                MetadataSerializer.SetMetadata(newItem, "DocumentPath", documentPath);
             }
 
             Outputs = outputs.ToArray();

src/Mvc/Extensions.ApiDescription.Design/src/build/Microsoft.Extensions.ApiDescription.Design.props

@@ -91,7 +91,7 @@
       -->
       <DocumentName />
       <!--
-        Full path where the API description document is placed. Default filename is %(Filename).%(DocumentName).json.
+        Full path where the API description document is placed. Default filename is %(Filename)_%(DocumentName).json.
         Filenames and relative paths (if explicitly set) are combined with $(ServiceProjectReferenceDirectory).
       -->
       <DocumentPath />
@@ -116,7 +116,7 @@
     </ServiceUriReference>
 
     <ServiceFileReference>
-      <!-- Name of the class to generate. Defaults to %(Filename)Client but with an uppercase first letter. -->
+      <!-- Name of the class to generate. Defaults to match final segment of %(OutputPath). -->
       <ClassName />
       <!--
         Code generator to use. Required and must end with "CSharp" or "TypeScript" (the currently-supported target
@@ -130,7 +130,7 @@
       <Namespace />
       <!--
         Path to place generated code. Code generator may interpret path as a filename or directory. Default filename or
-        folder name is %(ClassName).[cs|ts]. Filenames and relative paths (if explicitly set) are combined with
+        folder name is %(Filename)Client.[cs|ts]. Filenames and relative paths (if explicitly set) are combined with
         $(ServiceFileReferenceDirectory). Final value (depending on $(ServiceFileReferenceDirectory)) is likely to be
         a path relative to the client project.
       -->

src/Mvc/Extensions.ApiDescription.Design/src/build/Microsoft.Extensions.ApiDescription.Design.targets

@@ -311,7 +311,7 @@
         <SourceDocument>%(ServiceFileReference.FullPath)</SourceDocument>
       </Compile>
 
-      <!-- Otherwise, add all descendent files with the expected extension. -->
+      <!-- Otherwise, add all descendant files with the expected extension. -->
       <TypeScriptCompile Remove="@(_Directories -> '%(Identity)/**/*.ts')" />
       <TypeScriptCompile Include="@(_Directories -> '%(Identity)/**/*.ts')">
         <SourceDocument>%(_Directories.FullPath)</SourceDocument>

src/Mvc/GetDocumentInsider/src/Commands/GetDocumentCommandWorker.cs

@@ -117,12 +117,14 @@ namespace Microsoft.Extensions.ApiDescription.Tool.Commands
                     }
 
                     writer.Flush();
-                    stream.Position = 0L;
+                    if (stream.Length > 0L)
-                    using (var outStream = File.Create(context.OutputPath))
                     {
-                        stream.CopyTo(outStream);
+                        stream.Position = 0L;
-
+                        using (var outStream = File.Create(context.OutputPath))
-                        outStream.Flush();
+                        {
+                            stream.CopyTo(outStream);
+                            outStream.Flush();
+                        }
                     }
                 }