Eliminating unnecessary public members (dotnet#345)

Directory.Packages.props

@@ -8,6 +8,7 @@
     <PackageVersion Include="Microsoft.Build.Framework" Version="17.3.1" />
     <PackageVersion Include="Microsoft.Build.Utilities.Core" Version="17.3.1" />
     <PackageVersion Include="Microsoft.Build" Version="17.3.1" />
+    <PackageVersion Include="Microsoft.CodeAnalysis.PublicApiAnalyzers" Version="3.3.4" />
     <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.4.1" />
     <PackageVersion Include="Microsoft.VisualStudioEng.MicroBuild.Core" Version="1.0.0" />
     <PackageVersion Include="Nerdbank.GitVersioning" Version="3.5.109" />

Microsoft.NET.Build.Containers.UnitTests/ContainerHelpersTests.cs

@@ -87,8 +87,8 @@ public void CanParsePort(string input, bool shouldParse, int? expectedPortNumber
 
         if (shouldParse) {
             Assert.NotNull(port);
-            Assert.Equal(port.number, expectedPortNumber);
-            Assert.Equal(port.type, expectedType);
+            Assert.Equal(port.Value.Number, expectedPortNumber);
+            Assert.Equal(port.Value.Type, expectedType);
         } else {
             Assert.Null(port);
             Assert.NotNull(errors);

Microsoft.NET.Build.Containers/AmazonECRMessageHandler.cs

@@ -11,7 +11,7 @@ namespace Microsoft.NET.Build.Containers;
 /// error message. The handler catches the generic error and provides a more informed error
 /// message to let the user know they need to create the repository.
 /// </summary>
-public class AmazonECRMessageHandler : DelegatingHandler
+internal sealed class AmazonECRMessageHandler : DelegatingHandler
 {
     public AmazonECRMessageHandler(HttpMessageHandler innerHandler) : base(innerHandler) { }
 

Microsoft.NET.Build.Containers/AuthHandshakeMessageHandler.cs

@@ -18,7 +18,7 @@ namespace Microsoft.NET.Build.Containers;
 /// <summary>
 /// A delegating handler that performs the Docker auth handshake as described <see href="https://docs.docker.com/registry/spec/auth/token/">in their docs</see> if a request isn't authenticated
 /// </summary>
-public partial class AuthHandshakeMessageHandler : DelegatingHandler
+internal sealed partial class AuthHandshakeMessageHandler : DelegatingHandler
 {
     private const int MaxRequestRetries = 5; // Arbitrary but seems to work ok for chunked uploads to ghcr.io
 

Microsoft.NET.Build.Containers/BaseImageNotFoundException.cs

@@ -3,8 +3,8 @@
 
 namespace Microsoft.NET.Build.Containers;
 
-public class BaseImageNotFoundException : Exception
+public sealed class BaseImageNotFoundException : Exception
 {
-    public BaseImageNotFoundException(string specifiedRuntimeIdentifier, string repositoryName, string reference, IEnumerable<string> supportedRuntimeIdentifiers)
+    internal BaseImageNotFoundException(string specifiedRuntimeIdentifier, string repositoryName, string reference, IEnumerable<string> supportedRuntimeIdentifiers)
             : base($"The RuntimeIdentifier '{specifiedRuntimeIdentifier}' is not supported by {repositoryName}:{reference}. The supported RuntimeIdentifiers are {String.Join(",", supportedRuntimeIdentifiers)}") {}
 }

Microsoft.NET.Build.Containers/ContainerBuilder.cs

@@ -1,22 +1,12 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-namespace Microsoft.NET.Build.Containers;
-
-using System;
-using System.IO;
 using System.Text.Json;
-using System.Threading.Tasks;
+
+namespace Microsoft.NET.Build.Containers;
 
 public static class ContainerBuilder
 {
-    private static LocalDocker GetLocalDaemon(string localDaemonType, Action<string> logger) {
-        var daemon = localDaemonType switch {
-            KnownDaemonTypes.Docker => new LocalDocker(logger),
-            _ => throw new ArgumentException($"Unknown local container daemon type '{localDaemonType}'. Valid local container daemon types are {String.Join(",", KnownDaemonTypes.SupportedLocalDaemonTypes)}", nameof(localDaemonType))
-        };
-        return daemon;
-    }
     public static async Task Containerize(DirectoryInfo folder, string workingDir, string registryName, string baseName, string baseTag, string[] entrypoint, string[] entrypointArgs, string imageName, string[] imageTags, string? outputRegistry, string[] labels, Port[] exposedPorts, string[] envVars, string containerRuntimeIdentifier, string ridGraphPath, string localContainerDaemon)
     {
         var isDaemonPull = String.IsNullOrEmpty(registryName);
@@ -104,4 +94,14 @@ public static async Task Containerize(DirectoryInfo folder, string workingDir, s
             }
         }
     }
+
+    private static LocalDocker GetLocalDaemon(string localDaemonType, Action<string> logger)
+    {
+        var daemon = localDaemonType switch
+        {
+            KnownDaemonTypes.Docker => new LocalDocker(logger),
+            _ => throw new ArgumentException($"Unknown local container daemon type '{localDaemonType}'. Valid local container daemon types are {String.Join(",", KnownDaemonTypes.SupportedLocalDaemonTypes)}", nameof(localDaemonType))
+        };
+        return daemon;
+    }
 }

Microsoft.NET.Build.Containers/ContainerHelpers.cs

@@ -1,31 +1,20 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-namespace Microsoft.NET.Build.Containers;
-
+#if NETFRAMEWORK
 using System;
+#endif
+#if NET
 using System.Diagnostics.CodeAnalysis;
+#endif
 using System.Text.RegularExpressions;
-using static ReferenceParser;
-
-internal sealed record Label(string name, string value);
-
-// Explicitly lowercase to ease parsing - the incoming values are
-// lowercased by spec
-public enum PortType
-{
-    tcp,
-    udp
-}
-
-public record Port(int number, PortType type);
-
 
+namespace Microsoft.NET.Build.Containers;
 public static class ContainerHelpers
 {
-    public const string HostObjectUser = "SDK_CONTAINER_REGISTRY_UNAME";
-    
-    public const string HostObjectPass = "SDK_CONTAINER_REGISTRY_PWORD";
+    internal const string HostObjectUser = "SDK_CONTAINER_REGISTRY_UNAME";
+
+    internal const string HostObjectPass = "SDK_CONTAINER_REGISTRY_PWORD";
 
     /// <summary>
     /// Matches an environment variable name - must start with a letter or underscore, and can only contain letters, numbers, and underscores.
@@ -37,30 +26,119 @@ public static class ContainerHelpers
     /// that have no registry component.
     /// See <see href="https://github.com/distribution/distribution/blob/78b9c98c5c31c30d74f9acb7d96f98552f2cf78f/reference/normalize.go">normalize.go</see>.
     /// </summary>
-    public const string DefaultRegistry = "docker.io";
+    internal const string DefaultRegistry = "docker.io";
 
     /// <summary>
     /// Matches if the string is not lowercase or numeric, or ., _, or -.
     /// </summary>
     /// <remarks>Technically the period should be allowed as well, but due to inconsistent support between cloud providers we're removing it.</remarks>
     private static Regex imageNameCharacters = new Regex(@"[^a-z0-9_\-/]");
 
+
+    /// <summary>
+    /// The enum contains possible error reasons during port parsing using <see cref="TryParsePort(string, out Port?, out ParsePortError?)"/> or <see cref="TryParsePort(string?, string?, out Port?, out ParsePortError?)"/>.
+    /// </summary>
+    [Flags]
+    public enum ParsePortError
+    {
+        MissingPortNumber,
+        InvalidPortNumber,
+        InvalidPortType,
+        UnknownPortFormat
+    }
+
+    /// <summary>
+    /// Tries to parse the port from <paramref name="portNumber"/> and <paramref name="portType"/>.
+    /// </summary>
+    /// <param name="portNumber">The port number to parse.</param>
+    /// <param name="portType">The port type to parse (tcp or udp).</param>
+    /// <param name="port">Parsed port.</param>
+    /// <param name="error">The error occurred during parsing. Only returned when method returns <see langword=""="false"/>.</param>
+    /// <returns><see langword=""="true"/> when port was successfully parsed, <see langword=""="false"/> otherwise.</returns>
+    public static bool TryParsePort(string? portNumber, string? portType, [NotNullWhen(true)] out Port? port, [NotNullWhen(false)] out ParsePortError? error)
+    {
+        var portNo = 0;
+        error = null;
+        if (String.IsNullOrEmpty(portNumber))
+        {
+            error = ParsePortError.MissingPortNumber;
+        }
+        else if (!int.TryParse(portNumber, out portNo))
+        {
+            error = ParsePortError.InvalidPortNumber;
+        }
+
+        if (!Enum.TryParse<PortType>(portType, out PortType t))
+        {
+            if (portType is not null)
+            {
+                error = (error ?? ParsePortError.InvalidPortType) | ParsePortError.InvalidPortType;
+            }
+            else
+            {
+                t = PortType.tcp;
+            }
+        }
+
+        if (error is null)
+        {
+            port = new Port(portNo, t);
+            return true;
+        }
+        else
+        {
+            port = null;
+            return false;
+        }
+
+    }
+
+    /// <summary>
+    /// Tries to parse the port from <paramref name="input"/>.
+    /// </summary>
+    /// <param name="input">The port number to parse. Expected formats are: port number as int value, or value in format 'port number/port type' where
+    /// port type can be tcp or udp. If the port type is not present, it is assumed to be tcp.</param>
+    /// <param name="port">Parsed port.</param>
+    /// <param name="error">The error occurred during parsing. Only returned when method returns <see langword="false"/>.</param>
+    /// <returns><see langword="true"/> when port was successfully parsed, <see langword="false"/> otherwise.</returns>
+    public static bool TryParsePort(string input, [NotNullWhen(true)] out Port? port, [NotNullWhen(false)] out ParsePortError? error)
+    {
+        var parts = input.Split('/');
+        if (parts.Length == 2)
+        {
+            string portNumber = parts[0];
+            string type = parts[1];
+            return TryParsePort(portNumber, type, out port, out error);
+        }
+        else if (parts.Length == 1)
+        {
+            string portNum = parts[0];
+            return TryParsePort(portNum, null, out port, out error);
+        }
+        else
+        {
+            error = ParsePortError.UnknownPortFormat;
+            port = null;
+            return false;
+        }
+    }
+
     /// <summary>
     /// Ensures the given registry is valid.
     /// </summary>
     /// <param name="registryName"></param>
     /// <returns></returns>
-    public static bool IsValidRegistry(string registryName) => AnchoredDomainRegexp.IsMatch(registryName);
+    internal static bool IsValidRegistry(string registryName) => ReferenceParser.AnchoredDomainRegexp.IsMatch(registryName);
 
     /// <summary>
     /// Ensures the given image name is valid.
     /// Spec: https://github.com/opencontainers/distribution-spec/blob/4ab4752c3b86a926d7e5da84de64cbbdcc18d313/spec.md#pulling-manifests
     /// </summary>
     /// <param name="imageName"></param>
     /// <returns></returns>
-    public static bool IsValidImageName(string imageName)
+    internal static bool IsValidImageName(string imageName)
     {
-        return anchoredNameRegexp.IsMatch(imageName);
+        return ReferenceParser.anchoredNameRegexp.IsMatch(imageName);
     }
 
     /// <summary>
@@ -69,16 +147,16 @@ public static bool IsValidImageName(string imageName)
     /// </summary>
     /// <param name="imageTag"></param>
     /// <returns></returns>
-    public static bool IsValidImageTag(string imageTag)
+    internal static bool IsValidImageTag(string imageTag)
     {
-        return anchoredTagRegexp.IsMatch(imageTag);
+        return ReferenceParser.anchoredTagRegexp.IsMatch(imageTag);
     }
 
     /// <summary>
     /// Given an already-validated registry domain, this is our hueristic to determine what HTTP protocol should be used to interact with it.
     /// This is primarily for testing - in the real world almost all usage should be through HTTPS!
     /// </summary>
-    public static Uri TryExpandRegistryToUri(string alreadyValidatedDomain)
+    internal static Uri TryExpandRegistryToUri(string alreadyValidatedDomain)
     {
         var prefix = alreadyValidatedDomain.StartsWith("localhost", StringComparison.Ordinal) ? "http" : "https";
         return new Uri($"{prefix}://{alreadyValidatedDomain}");
@@ -89,7 +167,7 @@ public static Uri TryExpandRegistryToUri(string alreadyValidatedDomain)
     /// </summary>
     /// <param name="envVar"></param>
     /// <returns></returns>
-    public static bool IsValidEnvironmentVariable(string envVar)
+    internal static bool IsValidEnvironmentVariable(string envVar)
     {
         return envVarRegex.IsMatch(envVar);
     }
@@ -109,7 +187,7 @@ public static bool IsValidEnvironmentVariable(string envVar)
     /// <param name="containerTag"></param>
     /// <param name="containerDigest"></param>
     /// <returns>True if the parse was successful. When false is returned, all out vars are set to empty strings.</returns>
-    public static bool TryParseFullyQualifiedContainerName(string fullyQualifiedContainerName,
+    internal static bool TryParseFullyQualifiedContainerName(string fullyQualifiedContainerName,
                                                             [NotNullWhen(true)] out string? containerRegistry,
                                                             [NotNullWhen(true)] out string? containerName,
                                                             out string? containerTag, // tag is always optional - we can't guarantee anything here
@@ -118,7 +196,7 @@ out string? containerDigest // digest is always optional - we can't guarantee an
     {
 
         /// if we don't have a reference at all, bail out
-        var referenceMatch = ReferenceRegexp.Match(fullyQualifiedContainerName);
+        var referenceMatch = ReferenceParser.ReferenceRegexp.Match(fullyQualifiedContainerName);
         if (referenceMatch is not { Success: true })
         {
             containerRegistry = null;
@@ -136,7 +214,7 @@ out string? containerDigest // digest is always optional - we can't guarantee an
         // this will always be successful if the ReferenceRegexp matched, so it's safe to index into.
         var namePortion = referenceMatch.Groups[1].Value;
         // we try to decompose the reference name into registry and image name parts.
-        var nameMatch = anchoredNameRegexp.Match(namePortion);
+        var nameMatch = ReferenceParser.anchoredNameRegexp.Match(namePortion);
         if (nameMatch is { Success: true })
         {
             // the name regex has two groups:
@@ -183,7 +261,7 @@ out string? containerDigest // digest is always optional - we can't guarantee an
     /// <summary>
     /// Checks if a given container image name adheres to the image name spec. If not, and recoverable, then normalizes invalid characters.
     /// </summary>
-    public static bool NormalizeImageName(string containerImageName,
+    internal static bool NormalizeImageName(string containerImageName,
                                          [NotNullWhen(false)] out string? normalizedImageName)
     {
         if (IsValidImageName(containerImageName))
@@ -202,73 +280,4 @@ public static bool NormalizeImageName(string containerImageName,
             return false;
         }
     }
-
-    [Flags]
-    public enum ParsePortError
-    {
-        MissingPortNumber,
-        InvalidPortNumber,
-        InvalidPortType,
-        UnknownPortFormat
-    }
-
-    public static bool TryParsePort(string? portNumber, string? portType, [NotNullWhen(true)] out Port? port, [NotNullWhen(false)] out ParsePortError? error)
-    {
-        var portNo = 0;
-        error = null;
-        if (String.IsNullOrEmpty(portNumber))
-        {
-            error = ParsePortError.MissingPortNumber;
-        }
-        else if (!int.TryParse(portNumber, out portNo))
-        {
-            error = ParsePortError.InvalidPortNumber;
-        }
-
-        if (!Enum.TryParse<PortType>(portType, out PortType t))
-        {
-            if (portType is not null)
-            {
-                error = (error ?? ParsePortError.InvalidPortType) | ParsePortError.InvalidPortType;
-            }
-            else
-            {
-                t = PortType.tcp;
-            }
-        }
-
-        if (error is null)
-        {
-            port = new Port(portNo, t);
-            return true;
-        }
-        else
-        {
-            port = null;
-            return false;
-        }
-
-    }
-
-    public static bool TryParsePort(string input, [NotNullWhen(true)] out Port? port, [NotNullWhen(false)] out ParsePortError? error)
-    {
-        var parts = input.Split('/');
-        if (parts.Length == 2)
-        {
-            string portNumber = parts[0];
-            string type = parts[1];
-            return TryParsePort(portNumber, type, out port, out error);
-        }
-        else if (parts.Length == 1)
-        {
-            string portNum = parts[0];
-            return TryParsePort(portNum, null, out port, out error);
-        }
-        else
-        {
-            error = ParsePortError.UnknownPortFormat;
-            port = null;
-            return false;
-        }
-    }
 }