diff --git a/docs/coding-guidelines/adding-api-guidelines.md b/docs/coding-guidelines/adding-api-guidelines.md index 3854dca911f82d..a276c3af927b7e 100644 --- a/docs/coding-guidelines/adding-api-guidelines.md +++ b/docs/coding-guidelines/adding-api-guidelines.md @@ -58,6 +58,48 @@ The rest of the documentation workflow depends on whether the assembly has the ` - Triple-slash comments in source code are synced to dotnet-api-docs periodically (every preview). - More recently introduced libraries typically follow this workflow. +### Documentation placement in platform-specific libraries + +When a library targets platform-specific frameworks (e.g. `net11.0-windows`, `net11.0-linux`), +only **one** platform's compiler-generated doc XML is selected as the source of truth and shipped +to all customers in the IntelliSense package. This means that if XML doc comments for a public API +appear only in a platform-specific partial file, they may be missing from the shipped docs on other +platforms. + +To ensure consistent documentation across all platforms, follow these rules: + +1. **Place docs on the primary source file.** Each public type should have a primary source file + named `TypeName.cs`. All public API documentation (`/// `, `/// `, etc.) must + be placed in this file. +2. **Follow the naming convention for partial files.** Platform-specific or feature-specific + partials must follow the `TypeName.Something.cs` naming convention (e.g. `Socket.Windows.cs`, + `Socket.Unix.cs`). +3. **Do not add public XML doc comments in non-primary partial files.** If a public member is + declared in a file like `TypeName.Windows.cs`, its documentation should be in `TypeName.cs` + (using a partial method declaration or ``), not in the platform-specific file. + +These rules are enforced by the **PlatformDocAnalyzer** (`eng/analyzers/PlatformDocAnalyzer`), +which is automatically applied to all library source projects. The analyzer only activates when +building a platform-specific target framework with `UseCompilerGeneratedDocXmlFile=true`, and +produces the following diagnostics: + +| Diagnostic | Description | +|------------|-------------| +| PLATDOC001 | Public type has no source file named `TypeName.cs`. | +| PLATDOC002 | Partial source file doesn't follow the `TypeName.Something.cs` naming convention. | +| PLATDOC003 | Public member in a non-primary partial file has XML documentation that should be moved to `TypeName.cs`. | +| PLATDOC004 | Documentation for a public API differs from the canonical (platform-agnostic) build. | + +PLATDOC001–003 are heuristic rules that guide source organization. PLATDOC004 is an authoritative +check: when a project also targets a platform-agnostic TFM (e.g. `net11.0` alongside +`net11.0-windows`), the build passes the canonical TFM's compiler-generated doc XML to the +analyzer, which compares each public API's documentation against it. Any mismatch indicates that +docs were placed on platform-specific source and will be inconsistent across platforms. + +If a file legitimately doesn't follow these conventions (e.g. an `Async` partial using the +established `TypeNameAsync.cs` pattern), suppress the specific diagnostic with +`#pragma warning disable PLATDOCnnn` and a brief comment explaining why. + **For libraries with `false`:** - The [dotnet-api-docs](https://github.com/dotnet/dotnet-api-docs) repo is the source of truth for documentation. - Triple-slash comments in source code are synced to dotnet-api-docs **only once** for newly introduced APIs. After the initial sync, all subsequent documentation diff --git a/eng/analyzers/Directory.Build.props b/eng/analyzers/Directory.Build.props new file mode 100644 index 00000000000000..003cb8850bdc7c --- /dev/null +++ b/eng/analyzers/Directory.Build.props @@ -0,0 +1,8 @@ + + + + true + + + + diff --git a/eng/analyzers/PlatformDocAnalyzer.Tests/PlatformDocAnalyzer.Tests.csproj b/eng/analyzers/PlatformDocAnalyzer.Tests/PlatformDocAnalyzer.Tests.csproj new file mode 100644 index 00000000000000..95e217d02037ec --- /dev/null +++ b/eng/analyzers/PlatformDocAnalyzer.Tests/PlatformDocAnalyzer.Tests.csproj @@ -0,0 +1,16 @@ + + + + $(NetCoreAppCurrent) + + + + + + + + + + + + diff --git a/eng/analyzers/PlatformDocAnalyzer.Tests/PlatformDocAnalyzerTests.cs b/eng/analyzers/PlatformDocAnalyzer.Tests/PlatformDocAnalyzerTests.cs new file mode 100644 index 00000000000000..6589200d80d811 --- /dev/null +++ b/eng/analyzers/PlatformDocAnalyzer.Tests/PlatformDocAnalyzerTests.cs @@ -0,0 +1,511 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Immutable; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Testing; +using Microsoft.DotNet.Analyzers.PlatformDoc; +using Xunit; + +namespace PlatformDocAnalyzer.Tests +{ + using AnalyzerTest = CSharpAnalyzerTest; + + public class PlatformDocAnalyzerTests + { + private static readonly (string Key, string Value)[] s_platformTfmOptions = new[] + { + ("build_property.TargetFramework", "net10.0-windows"), + ("build_property.UseCompilerGeneratedDocXmlFile", "true"), + }; + + private static readonly (string Key, string Value)[] s_nonPlatformTfmOptions = new[] + { + ("build_property.TargetFramework", "net10.0"), + ("build_property.UseCompilerGeneratedDocXmlFile", "true"), + }; + + private static AnalyzerTest CreateTest( + (string Key, string Value)[] globalOptions, + params (string FileName, string Source)[] sources) + { + var test = new AnalyzerTest + { + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }; + + foreach ((string fileName, string source) in sources) + { + test.TestState.Sources.Add((fileName, source)); + } + + string optionsText = "is_global = true\r\n"; + foreach ((string key, string value) in globalOptions) + { + optionsText += $"{key} = {value}\r\n"; + } + + test.TestState.AnalyzerConfigFiles.Add(("/.globalconfig", optionsText)); + return test; + } + + [Fact] + public async Task NoDiagnosticForNonPlatformTfm() + { + // A non-platform TFM should not trigger any diagnostics even if naming is wrong. + var test = CreateTest( + s_nonPlatformTfmOptions, + ("WrongName.cs", @" +public class Foo +{ + /// Some docs + public void Bar() { } +}")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task NoDiagnosticWhenUseCompilerGeneratedDocXmlFileIsFalse() + { + var options = new[] + { + ("build_property.TargetFramework", "net10.0-windows"), + ("build_property.UseCompilerGeneratedDocXmlFile", "false"), + }; + + var test = CreateTest( + options, + ("WrongName.cs", @" +public class Foo +{ + /// Some docs + public void Bar() { } +}")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task NoDiagnosticForCorrectSetup() + { + // Primary file matches type name, no docs on partial files. + var test = CreateTest( + s_platformTfmOptions, + ("Foo.cs", @" +/// Foo type +public partial class Foo +{ + /// Bar method + public void Bar() { } +}"), + ("Foo.Windows.cs", @" +public partial class Foo +{ + public void PlatformSpecificMethod() { } +}")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task PLATDOC001_PublicTypeMissingPrimaryFile() + { + var test = CreateTest( + s_platformTfmOptions, + ("Foo.Windows.cs", @" +public partial class {|#0:Foo|} +{ + public void Bar() { } +}"), + ("Foo.Unix.cs", @" +public partial class {|#1:Foo|} +{ + public void Baz() { } +}")); + + test.ExpectedDiagnostics.Add(new DiagnosticResult(Microsoft.DotNet.Analyzers.PlatformDoc.PlatformDocAnalyzer.MissingPrimaryFileRule) + .WithLocation(0).WithArguments("Foo")); + test.ExpectedDiagnostics.Add(new DiagnosticResult(Microsoft.DotNet.Analyzers.PlatformDoc.PlatformDocAnalyzer.MissingPrimaryFileRule) + .WithLocation(1).WithArguments("Foo")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task PLATDOC002_BadPartialFileName() + { + var test = CreateTest( + s_platformTfmOptions, + ("Foo.cs", @" +/// Foo type +public partial class Foo +{ + /// Bar method + public void Bar() { } +}"), + ("Helpers.cs", @" +public partial class {|#0:Foo|} +{ + public void PlatformSpecificHelper() { } +}")); + + test.ExpectedDiagnostics.Add(new DiagnosticResult(Microsoft.DotNet.Analyzers.PlatformDoc.PlatformDocAnalyzer.BadPartialFileNameRule) + .WithLocation(0).WithArguments("Helpers.cs", "Foo")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task PLATDOC003_DocsOnNonPrimaryFile() + { + var test = CreateTest( + s_platformTfmOptions, + ("Foo.cs", @" +/// Foo type +public partial class Foo +{ + /// Bar method + public void Bar() { } +}"), + ("Foo.Windows.cs", @" +public partial class Foo +{ + /// This doc should be in Foo.cs + public void {|#0:PlatformSpecificMethod|}() { } +}")); + + test.ExpectedDiagnostics.Add(new DiagnosticResult(Microsoft.DotNet.Analyzers.PlatformDoc.PlatformDocAnalyzer.DocsOnNonPrimaryFileRule) + .WithLocation(0).WithArguments("PlatformSpecificMethod", "Foo.Windows.cs", "Foo")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task PLATDOC003_DocsOnNonPrimaryFile_MultipleMembers() + { + var test = CreateTest( + s_platformTfmOptions, + ("MyService.cs", @" +/// MyService type +public partial class MyService +{ + /// Start method + public void Start() { } +}"), + ("MyService.Windows.cs", @" +public partial class MyService +{ + /// Windows-specific start + public void {|#0:StartWindows|}() { } + + /// Windows handle + public int {|#1:Handle|} { get; set; } + + // No docs - this is fine + public void InternalHelper() { } +}")); + + test.ExpectedDiagnostics.Add(new DiagnosticResult(Microsoft.DotNet.Analyzers.PlatformDoc.PlatformDocAnalyzer.DocsOnNonPrimaryFileRule) + .WithLocation(0).WithArguments("StartWindows", "MyService.Windows.cs", "MyService")); + test.ExpectedDiagnostics.Add(new DiagnosticResult(Microsoft.DotNet.Analyzers.PlatformDoc.PlatformDocAnalyzer.DocsOnNonPrimaryFileRule) + .WithLocation(1).WithArguments("Handle", "MyService.Windows.cs", "MyService")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task NoDiagnosticForNonPublicType() + { + var test = CreateTest( + s_platformTfmOptions, + ("WrongName.cs", @" +internal class Foo +{ + /// Some docs + public void Bar() { } +}")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task NoDiagnosticForPrivateMemberDocs() + { + // Private/internal members in non-primary files can have docs + var test = CreateTest( + s_platformTfmOptions, + ("Foo.cs", @" +/// Foo type +public partial class Foo +{ + /// Bar method + public void Bar() { } +}"), + ("Foo.Windows.cs", @" +public partial class Foo +{ + /// Private helper - docs are fine here + private void PrivateHelper() { } + + /// Internal helper - docs are fine here + internal void InternalHelper() { } +}")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task NoDiagnosticForNestedType() + { + // Nested types are not checked (only top-level types are) + var test = CreateTest( + s_platformTfmOptions, + ("Outer.cs", @" +/// Outer type +public class Outer +{ + /// Inner type + public class Inner + { + /// Method + public void Method() { } + } +}")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task PLATDOC003_InterfaceMembersAreImplicitlyPublic() + { + var test = CreateTest( + s_platformTfmOptions, + ("IService.cs", @" +/// IService interface +public partial interface IService +{ + /// Start method + void Start(); +}"), + ("IService.Windows.cs", @" +public partial interface IService +{ + /// Windows-specific method + void {|#0:StartWindows|}(); +}")); + + test.ExpectedDiagnostics.Add(new DiagnosticResult(Microsoft.DotNet.Analyzers.PlatformDoc.PlatformDocAnalyzer.DocsOnNonPrimaryFileRule) + .WithLocation(0).WithArguments("StartWindows", "IService.Windows.cs", "IService")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task PLATDOC001_StructType() + { + var test = CreateTest( + s_platformTfmOptions, + ("WrongName.cs", @" +public partial struct {|#0:MyStruct|} +{ + public int Value; +}"), + ("WrongName2.cs", @" +public partial struct {|#1:MyStruct|} +{ + public int Value2; +}")); + + test.ExpectedDiagnostics.Add(new DiagnosticResult(Microsoft.DotNet.Analyzers.PlatformDoc.PlatformDocAnalyzer.MissingPrimaryFileRule) + .WithLocation(0).WithArguments("MyStruct")); + test.ExpectedDiagnostics.Add(new DiagnosticResult(Microsoft.DotNet.Analyzers.PlatformDoc.PlatformDocAnalyzer.MissingPrimaryFileRule) + .WithLocation(1).WithArguments("MyStruct")); + test.ExpectedDiagnostics.Add(new DiagnosticResult(Microsoft.DotNet.Analyzers.PlatformDoc.PlatformDocAnalyzer.BadPartialFileNameRule) + .WithLocation(0).WithArguments("WrongName.cs", "MyStruct")); + test.ExpectedDiagnostics.Add(new DiagnosticResult(Microsoft.DotNet.Analyzers.PlatformDoc.PlatformDocAnalyzer.BadPartialFileNameRule) + .WithLocation(1).WithArguments("WrongName2.cs", "MyStruct")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task NoDiagnosticForSingleFileType() + { + // A type defined in only one file should not trigger any diagnostics, + // even if the file name doesn't match the type name. + var test = CreateTest( + s_platformTfmOptions, + ("Helpers.cs", @" +/// Some type +public class Foo +{ + /// Some method + public void Bar() { } +}")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task NoDiagnosticForNestedTypeDocsOnNonPrimaryFile() + { + // Nested type declarations define their own doc scope and should not be flagged. + var test = CreateTest( + s_platformTfmOptions, + ("Outer.cs", @" +/// Outer type +public partial class Outer +{ + /// Method + public void Method() { } +}"), + ("Outer.Inner.cs", @" +public partial class Outer +{ + /// Nested type docs are fine here + public class Inner + { + /// Inner method + public void InnerMethod() { } + } +}")); + + await test.RunAsync(CancellationToken.None); + } + + private static AnalyzerTest CreateTestWithCanonicalDoc( + (string Key, string Value)[] globalOptions, + string canonicalDocXml, + params (string FileName, string Source)[] sources) + { + var test = CreateTest(globalOptions, sources); + + // Replace the existing globalconfig with one that also configures the additional file. + test.TestState.AnalyzerConfigFiles.Clear(); + string optionsText = "is_global = true\r\n"; + foreach ((string key, string value) in globalOptions) + { + optionsText += $"{key} = {value}\r\n"; + } + optionsText += "[/canonical.xml]\r\n"; + optionsText += "build_metadata.AdditionalFiles.PlatformDocCanonical = true\r\n"; + test.TestState.AnalyzerConfigFiles.Add(("/.globalconfig", optionsText)); + + test.TestState.AdditionalFiles.Add(("/canonical.xml", canonicalDocXml)); + return test; + } + + [Fact] + public async Task PLATDOC004_DocsMatchCanonical_NoDiagnostic() + { + string canonicalDocXml = @" + + TestAssembly + + + Foo type + + + Bar method + + +"; + + var test = CreateTestWithCanonicalDoc( + s_platformTfmOptions, + canonicalDocXml, + ("Foo.cs", @" +/// Foo type +public class Foo +{ + /// Bar method + public void Bar() { } +}")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task PLATDOC004_DocsDifferFromCanonical() + { + string canonicalDocXml = @" + + TestAssembly + + + Foo type + + + Bar method + + +"; + + var test = CreateTestWithCanonicalDoc( + s_platformTfmOptions, + canonicalDocXml, + ("Foo.cs", @" +/// Foo type +public class Foo +{ + /// DIFFERENT Bar method docs + public void {|#0:Bar|}() { } +}")); + + test.ExpectedDiagnostics.Add(new DiagnosticResult(Microsoft.DotNet.Analyzers.PlatformDoc.PlatformDocAnalyzer.DocMismatchRule) + .WithLocation(0).WithArguments("Foo.Bar()")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task PLATDOC004_PlatformOnlyApi_NoDiagnostic() + { + // APIs that don't exist in the canonical doc XML are platform-specific and can have any docs. + string canonicalDocXml = @" + + TestAssembly + + + Foo type + + +"; + + var test = CreateTestWithCanonicalDoc( + s_platformTfmOptions, + canonicalDocXml, + ("Foo.cs", @" +/// Foo type +public class Foo +{ + /// Platform-specific method not in canonical + public void WindowsOnly() { } +}")); + + await test.RunAsync(CancellationToken.None); + } + + [Fact] + public async Task PLATDOC004_NoCanonicalDoc_NoDiagnostic() + { + // When no canonical doc XML is provided, PLATDOC004 should not fire. + var test = CreateTest( + s_platformTfmOptions, + ("Foo.cs", @" +/// DIFFERENT docs +public class Foo +{ + /// Some method + public void Bar() { } +}")); + + await test.RunAsync(CancellationToken.None); + } + } +} diff --git a/eng/analyzers/PlatformDocAnalyzer/PlatformDocAnalyzer.cs b/eng/analyzers/PlatformDocAnalyzer/PlatformDocAnalyzer.cs new file mode 100644 index 00000000000000..e2d3a7b081339d --- /dev/null +++ b/eng/analyzers/PlatformDocAnalyzer/PlatformDocAnalyzer.cs @@ -0,0 +1,428 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.IO; +using System.Linq; +using System.Text.RegularExpressions; +using System.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Text; + +#pragma warning disable RS2008 // Not a shipping analyzer + +namespace Microsoft.DotNet.Analyzers.PlatformDoc +{ + /// + /// Ensures platform-specific libraries with UseCompilerGeneratedDocXmlFile=true + /// place their triple-slash documentation on the primary source file so that + /// docs are consistent across platforms. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class PlatformDocAnalyzer : DiagnosticAnalyzer + { + private static readonly Regex s_memberRegex = new(@"(.*?)", RegexOptions.Singleline | RegexOptions.Compiled); + private static readonly Regex s_whitespaceRegex = new(@"\s+", RegexOptions.Compiled); + + public const string DiagnosticIdMissingPrimaryFile = "PLATDOC001"; + public const string DiagnosticIdBadPartialFileName = "PLATDOC002"; + public const string DiagnosticIdDocsOnNonPrimaryFile = "PLATDOC003"; + public const string DiagnosticIdDocMismatch = "PLATDOC004"; + + public static readonly DiagnosticDescriptor MissingPrimaryFileRule = new( + DiagnosticIdMissingPrimaryFile, + "Public type missing primary source file", + "Public type '{0}' has no source file named '{0}.cs'", + "Documentation", + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + public static readonly DiagnosticDescriptor BadPartialFileNameRule = new( + DiagnosticIdBadPartialFileName, + "Partial source file doesn't follow naming convention", + "Source file '{0}' contains a partial definition of type '{1}' but doesn't follow the '{1}.*.cs' naming convention", + "Documentation", + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + public static readonly DiagnosticDescriptor DocsOnNonPrimaryFileRule = new( + DiagnosticIdDocsOnNonPrimaryFile, + "XML documentation on non-primary partial file", + "Public member '{0}' in file '{1}' has XML documentation that should be moved to '{2}.cs'", + "Documentation", + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + public static readonly DiagnosticDescriptor DocMismatchRule = new( + DiagnosticIdDocMismatch, + "Documentation differs from canonical platform-agnostic build", + "Documentation for '{0}' differs from the canonical (platform-agnostic) build. Ensure docs are on shared source so they are consistent across platforms.", + "Documentation", + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + public override ImmutableArray SupportedDiagnostics => + ImmutableArray.Create(MissingPrimaryFileRule, BadPartialFileNameRule, DocsOnNonPrimaryFileRule, DocMismatchRule); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private static void OnCompilationStart(CompilationStartAnalysisContext context) + { + var options = context.Options.AnalyzerConfigOptionsProvider.GlobalOptions; + + if (!options.TryGetValue("build_property.TargetFramework", out string? tfm) || + string.IsNullOrEmpty(tfm) || + !IsPlatformSpecificTfm(tfm)) + { + return; + } + + if (!options.TryGetValue("build_property.UseCompilerGeneratedDocXmlFile", out string? useCompilerDoc) || + !string.Equals(useCompilerDoc, "true", StringComparison.OrdinalIgnoreCase)) + { + return; + } + + context.RegisterSymbolAction(AnalyzeNamedType, SymbolKind.NamedType); + + // Look for the canonical doc XML in AdditionalFiles. + Dictionary? canonicalDocs = TryLoadCanonicalDocs(context); + if (canonicalDocs is not null) + { + context.RegisterSymbolAction( + ctx => AnalyzeDocConsistency(ctx, canonicalDocs), + SymbolKind.NamedType); + } + } + + private static Dictionary? TryLoadCanonicalDocs(CompilationStartAnalysisContext context) + { + foreach (AdditionalText file in context.Options.AdditionalFiles) + { + AnalyzerConfigOptions fileOptions = context.Options.AnalyzerConfigOptionsProvider.GetOptions(file); + if (!fileOptions.TryGetValue("build_metadata.AdditionalFiles.PlatformDocCanonical", out string? value) || + !string.Equals(value, "true", StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + SourceText? text = file.GetText(context.CancellationToken); + if (text is null) + continue; + + return ParseDocXml(text.ToString()); + } + + return null; + } + + private static Dictionary ParseDocXml(string xml) + { + var docs = new Dictionary(StringComparer.Ordinal); + + // Use regex to extract member elements to avoid XML parser normalization + // that could cause false mismatches with GetDocumentationCommentXml() output. + foreach (Match match in s_memberRegex.Matches(xml)) + { + string name = match.Groups[1].Value; + string innerXml = match.Groups[2].Value; + docs[name] = NormalizeDocXml(innerXml); + } + + return docs; + } + + private static void AnalyzeDocConsistency( + SymbolAnalysisContext context, + Dictionary canonicalDocs) + { + ISymbol symbol = context.Symbol; + + if (symbol is not INamedTypeSymbol namedType) + return; + + if (namedType.DeclaredAccessibility != Accessibility.Public) + return; + + // Check the type itself and all its public members. + CheckSymbolDoc(context, namedType, canonicalDocs); + + foreach (ISymbol member in namedType.GetMembers()) + { + if (member.DeclaredAccessibility != Accessibility.Public) + continue; + + // Skip accessors; they're covered by the property/event. + if (member is IMethodSymbol { AssociatedSymbol: not null }) + continue; + + // Skip nested types; they get their own SymbolKind.NamedType callback. + if (member is INamedTypeSymbol) + continue; + + CheckSymbolDoc(context, member, canonicalDocs); + } + } + + private static void CheckSymbolDoc( + SymbolAnalysisContext context, + ISymbol symbol, + Dictionary canonicalDocs) + { + string? docId = symbol.GetDocumentationCommentId(); + if (docId is null) + return; + + if (!canonicalDocs.TryGetValue(docId, out string? canonicalDoc)) + return; + + string currentDoc = NormalizeDocXml( + symbol.GetDocumentationCommentXml(expandIncludes: true, cancellationToken: context.CancellationToken) ?? ""); + + // Strip the outer wrapper from GetDocumentationCommentXml() output. + currentDoc = StripMemberWrapper(currentDoc); + + if (string.Equals(canonicalDoc, currentDoc, StringComparison.Ordinal)) + return; + + // Both empty → no mismatch. + if (string.IsNullOrWhiteSpace(canonicalDoc) && string.IsNullOrWhiteSpace(currentDoc)) + return; + + Location location = symbol.Locations.FirstOrDefault() ?? Location.None; + context.ReportDiagnostic(Diagnostic.Create( + DocMismatchRule, + location, + symbol.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat))); + } + + private static string StripMemberWrapper(string xml) + { + // GetDocumentationCommentXml() returns: + // ..inner.. + // We need just the inner content to compare against the parsed doc XML. + const string memberStart = "', xml.IndexOf(memberStart, StringComparison.Ordinal) + 1); + int endIdx = xml.LastIndexOf(memberEnd, StringComparison.Ordinal); + + if (startIdx < 0 || endIdx < 0 || endIdx <= startIdx) + return NormalizeDocXml(xml); + + return NormalizeDocXml(xml.Substring(startIdx + 1, endIdx - startIdx - 1)); + } + + private static string NormalizeDocXml(string xml) + { + // Normalize whitespace: collapse runs of whitespace into single spaces, trim. + return s_whitespaceRegex.Replace(xml, " ").Trim(); + } + + private static bool IsPlatformSpecificTfm(string tfm) + { + // Platform-specific TFMs have a platform suffix: net10.0-windows, net9.0-linux, etc. + int dashIndex = tfm.IndexOf('-'); + return dashIndex > 0; + } + + private static void AnalyzeNamedType(SymbolAnalysisContext context) + { + var namedType = (INamedTypeSymbol)context.Symbol; + + if (namedType.DeclaredAccessibility != Accessibility.Public) + return; + + // Only check top-level types, not nested types. + if (namedType.ContainingType is not null) + return; + + ImmutableArray syntaxRefs = namedType.DeclaringSyntaxReferences; + if (syntaxRefs.IsEmpty) + return; + + // Only check types declared across multiple files. + // A type in a single file doesn't have a doc placement problem. + var distinctFiles = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (SyntaxReference syntaxRef in syntaxRefs) + { + distinctFiles.Add(syntaxRef.SyntaxTree.FilePath); + } + + if (distinctFiles.Count <= 1) + return; + + string typeName = namedType.Name; + string primaryFileName = typeName + ".cs"; + + bool hasPrimaryFile = false; + var nonPrimaryRefs = new List<(SyntaxReference SyntaxRef, string FileName)>(); + + foreach (SyntaxReference syntaxRef in syntaxRefs) + { + string filePath = syntaxRef.SyntaxTree.FilePath; + string fileName = Path.GetFileName(filePath); + + if (string.Equals(fileName, primaryFileName, StringComparison.OrdinalIgnoreCase)) + { + hasPrimaryFile = true; + } + else + { + nonPrimaryRefs.Add((syntaxRef, fileName)); + } + } + + // PLATDOC001: Public type must have a primary source file named TypeName.cs + if (!hasPrimaryFile) + { + foreach (SyntaxReference syntaxRef in syntaxRefs) + { + SyntaxNode node = syntaxRef.GetSyntax(context.CancellationToken); + if (node is TypeDeclarationSyntax typeDecl) + { + context.ReportDiagnostic(Diagnostic.Create( + MissingPrimaryFileRule, + typeDecl.Identifier.GetLocation(), + typeName)); + } + } + } + + foreach ((SyntaxReference syntaxRef, string fileName) in nonPrimaryRefs) + { + SyntaxNode node = syntaxRef.GetSyntax(context.CancellationToken); + if (node is not TypeDeclarationSyntax typeDecl) + continue; + + // PLATDOC002: Non-primary files must follow TypeName.Something.cs convention + string prefix = typeName + "."; + if (!fileName.StartsWith(prefix, StringComparison.OrdinalIgnoreCase) || + !fileName.EndsWith(".cs", StringComparison.OrdinalIgnoreCase) || + fileName.Length <= prefix.Length + ".cs".Length - 1) + { + context.ReportDiagnostic(Diagnostic.Create( + BadPartialFileNameRule, + typeDecl.Identifier.GetLocation(), + fileName, + typeName)); + } + + // PLATDOC003: Public members in non-primary files must not have XML docs + CheckMembersForDocs(context, typeDecl, fileName, typeName); + } + } + + private static void CheckMembersForDocs( + SymbolAnalysisContext context, + TypeDeclarationSyntax typeDecl, + string fileName, + string typeName) + { + foreach (MemberDeclarationSyntax member in typeDecl.Members) + { + // Nested type declarations define their own doc scope; skip them. + if (member is BaseTypeDeclarationSyntax or DelegateDeclarationSyntax) + continue; + + if (!IsEffectivelyPublic(member, typeDecl)) + continue; + + if (HasXmlDocComment(member)) + { + string memberName = GetMemberName(member); + context.ReportDiagnostic(Diagnostic.Create( + DocsOnNonPrimaryFileRule, + GetMemberIdentifierLocation(member), + memberName, + fileName, + typeName)); + } + } + } + + private static bool IsEffectivelyPublic(MemberDeclarationSyntax member, TypeDeclarationSyntax containingType) + { + // Interface members without explicit access modifiers are implicitly public. + // C# 8+ allows private/protected/internal members in interfaces. + if (containingType is InterfaceDeclarationSyntax) + { + if (member.Modifiers.Count == 0) + return true; + + return !member.Modifiers.Any(SyntaxKind.PrivateKeyword) && + !member.Modifiers.Any(SyntaxKind.ProtectedKeyword) && + !member.Modifiers.Any(SyntaxKind.InternalKeyword); + } + + // Enum members are implicitly public + if (member is EnumMemberDeclarationSyntax) + return true; + + return member.Modifiers.Any(SyntaxKind.PublicKeyword); + } + + private static bool HasXmlDocComment(MemberDeclarationSyntax member) + { + foreach (SyntaxTrivia trivia in member.GetLeadingTrivia()) + { + SyntaxKind kind = trivia.Kind(); + if (kind == SyntaxKind.SingleLineDocumentationCommentTrivia || + kind == SyntaxKind.MultiLineDocumentationCommentTrivia) + { + return true; + } + } + + return false; + } + + private static Location GetMemberIdentifierLocation(MemberDeclarationSyntax member) + { + return member switch + { + MethodDeclarationSyntax m => m.Identifier.GetLocation(), + PropertyDeclarationSyntax p => p.Identifier.GetLocation(), + EventDeclarationSyntax e => e.Identifier.GetLocation(), + EventFieldDeclarationSyntax ef => ef.Declaration.Variables.FirstOrDefault()?.Identifier.GetLocation() ?? member.GetLocation(), + FieldDeclarationSyntax f => f.Declaration.Variables.FirstOrDefault()?.Identifier.GetLocation() ?? member.GetLocation(), + ConstructorDeclarationSyntax c => c.Identifier.GetLocation(), + DestructorDeclarationSyntax d => d.Identifier.GetLocation(), + IndexerDeclarationSyntax i => i.ThisKeyword.GetLocation(), + OperatorDeclarationSyntax o => o.OperatorToken.GetLocation(), + ConversionOperatorDeclarationSyntax c => c.Type.GetLocation(), + EnumMemberDeclarationSyntax e => e.Identifier.GetLocation(), + _ => member.GetLocation() + }; + } + + private static string GetMemberName(MemberDeclarationSyntax member) + { + return member switch + { + MethodDeclarationSyntax m => m.Identifier.Text, + PropertyDeclarationSyntax p => p.Identifier.Text, + EventDeclarationSyntax e => e.Identifier.Text, + EventFieldDeclarationSyntax ef => ef.Declaration.Variables.FirstOrDefault()?.Identifier.Text ?? "", + FieldDeclarationSyntax f => f.Declaration.Variables.FirstOrDefault()?.Identifier.Text ?? "", + ConstructorDeclarationSyntax c => c.Identifier.Text, + DestructorDeclarationSyntax d => "~" + d.Identifier.Text, + IndexerDeclarationSyntax => "this[]", + OperatorDeclarationSyntax o => "operator " + o.OperatorToken.Text, + ConversionOperatorDeclarationSyntax c => c.ImplicitOrExplicitKeyword.Text + " operator " + c.Type, + EnumMemberDeclarationSyntax e => e.Identifier.Text, + _ => "" + }; + } + } +} diff --git a/eng/analyzers/PlatformDocAnalyzer/PlatformDocAnalyzer.csproj b/eng/analyzers/PlatformDocAnalyzer/PlatformDocAnalyzer.csproj new file mode 100644 index 00000000000000..f16a96838fc102 --- /dev/null +++ b/eng/analyzers/PlatformDocAnalyzer/PlatformDocAnalyzer.csproj @@ -0,0 +1,18 @@ + + + + netstandard2.0 + true + latest + enable + + $(NoWarn);RS2008;RS1012;RS1038 + cs + true + + + + + + + diff --git a/eng/analyzers/PlatformDocAnalyzer/PlatformDocAnalyzer.props b/eng/analyzers/PlatformDocAnalyzer/PlatformDocAnalyzer.props new file mode 100644 index 00000000000000..83923f3105f339 --- /dev/null +++ b/eng/analyzers/PlatformDocAnalyzer/PlatformDocAnalyzer.props @@ -0,0 +1,7 @@ + + + + + + + diff --git a/eng/analyzers/README.md b/eng/analyzers/README.md new file mode 100644 index 00000000000000..067451c86c3818 --- /dev/null +++ b/eng/analyzers/README.md @@ -0,0 +1,14 @@ +# PlatformDocAnalyzer + +A Roslyn analyzer that enforces documentation placement conventions for platform-specific +libraries with `UseCompilerGeneratedDocXmlFile=true`. + +## Running tests + +``` +dotnet test eng/analyzers/PlatformDocAnalyzer.Tests/PlatformDocAnalyzer.Tests.csproj +``` + +These tests are not part of the main CI test pipeline (consistent with other infrastructure +analyzer tests like `IntrinsicsInSystemPrivateCoreLibAnalyzer.Tests`). Run them locally when +modifying the analyzer. diff --git a/eng/generators.targets b/eng/generators.targets index 3290f37580be29..08d445786c8647 100644 --- a/eng/generators.targets +++ b/eng/generators.targets @@ -93,4 +93,15 @@ + + + + + + + diff --git a/eng/intellisense.targets b/eng/intellisense.targets index 1de45098a8fa0a..25fc2e05bd659b 100644 --- a/eng/intellisense.targets +++ b/eng/intellisense.targets @@ -87,70 +87,104 @@ - - - + + + + + - + - - + + <_StrippedTfm>$(TargetFramework.Substring(0, $(TargetFramework.IndexOf('-')))) + + + + + + + + + + + + + - + - @(_PNSECandidateTargetFramework) + @(_StrippedTfmCandidate) + @(_NonPlatformDocCandidate) + @(_AnyDocCandidate) + + - - + - - - - - + Condition="'$(UseCompilerGeneratedDocXmlFile)' == 'true' and + '$(CanonicalDocSourceTargetFramework)' != '' and + '$(CanonicalDocSourceTargetFramework)' != '$(TargetFramework)' and + ('$(BuildTargetFramework)' == '' or '$(TargetFrameworkIdentifier)' == '.NETCoreApp')"> + + + OutputItemType="CanonicalDocSource" /> - + - - - @(PNSEDocSource->'%(DocFileItem)') + Condition="'@(CanonicalDocSource)' != ''"> + + + @(CanonicalDocSource->'%(DocFileItem)') + + + + + - + \ No newline at end of file diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Core/XmlReaderAsync.cs b/src/libraries/System.Private.Xml/src/System/Xml/Core/XmlReaderAsync.cs index f4eae45cba12e9..6c2239f902409f 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/Core/XmlReaderAsync.cs +++ b/src/libraries/System.Private.Xml/src/System/Xml/Core/XmlReaderAsync.cs @@ -10,9 +10,10 @@ namespace System.Xml { - // Represents a reader that provides fast, non-cached forward only stream access to XML data. +#pragma warning disable PLATDOC002 // Async partial uses established TypeNameAsync.cs naming convention [DebuggerDisplay($"{{{nameof(DebuggerDisplayProxy)}}}")] public abstract partial class XmlReader : IDisposable +#pragma warning restore PLATDOC002 { public virtual Task GetValueAsync() { diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Core/XmlWriterAsync.cs b/src/libraries/System.Private.Xml/src/System/Xml/Core/XmlWriterAsync.cs index 6bb887db392dc7..ab6cefb051647f 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/Core/XmlWriterAsync.cs +++ b/src/libraries/System.Private.Xml/src/System/Xml/Core/XmlWriterAsync.cs @@ -8,9 +8,9 @@ namespace System.Xml { - // Represents a writer that provides fast non-cached forward-only way of generating XML streams containing XML documents - // that conform to the W3C Extensible Markup Language (XML) 1.0 specification and the Namespaces in XML specification. +#pragma warning disable PLATDOC002 // Async partial uses established TypeNameAsync.cs naming convention public abstract partial class XmlWriter : IDisposable, IAsyncDisposable +#pragma warning restore PLATDOC002 { // Write methods // Writes out the XML declaration with the version "1.0". diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Resolvers/XmlPreloadedResolverAsync.cs b/src/libraries/System.Private.Xml/src/System/Xml/Resolvers/XmlPreloadedResolverAsync.cs index 09f53c552b5391..c2b80bb6fdab01 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/Resolvers/XmlPreloadedResolverAsync.cs +++ b/src/libraries/System.Private.Xml/src/System/Xml/Resolvers/XmlPreloadedResolverAsync.cs @@ -7,12 +7,9 @@ namespace System.Xml.Resolvers { - // - // XmlPreloadedResolver is an XmlResolver that which can be pre-loaded with data. - // By default it contains well-known DTDs for XHTML 1.0 and RSS 0.91. - // Custom mappings of URIs to data can be added with the Add method. - // +#pragma warning disable PLATDOC002 // Async partial uses established TypeNameAsync.cs naming convention public partial class XmlPreloadedResolver : XmlResolver +#pragma warning restore PLATDOC002 { public override Task GetEntityAsync(Uri absoluteUri, string? role, diff --git a/src/libraries/System.Private.Xml/src/System/Xml/XmlResolver.FileSystemResolver.cs b/src/libraries/System.Private.Xml/src/System/Xml/XmlResolver.FileSystemResolver.cs index 8785d09e9c12cb..e61e4082e7a4aa 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/XmlResolver.FileSystemResolver.cs +++ b/src/libraries/System.Private.Xml/src/System/Xml/XmlResolver.FileSystemResolver.cs @@ -8,6 +8,7 @@ namespace System.Xml { public abstract partial class XmlResolver { +#pragma warning disable PLATDOC003 // Not platform-specific; property lives in this file by convention /// /// Gets an XML resolver which resolves only file system URIs. /// @@ -17,6 +18,7 @@ public abstract partial class XmlResolver /// instance returned by this property will resolve only URIs which scheme is file. /// public static XmlResolver FileSystemResolver => XmlFileSystemResolver.s_singleton; +#pragma warning restore PLATDOC003 // An XML resolver that resolves only file system URIs. private sealed class XmlFileSystemResolver : XmlResolver diff --git a/src/libraries/System.Private.Xml/src/System/Xml/XmlResolver.ThrowingResolver.cs b/src/libraries/System.Private.Xml/src/System/Xml/XmlResolver.ThrowingResolver.cs index 674ddb630e0896..9fada155540a11 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/XmlResolver.ThrowingResolver.cs +++ b/src/libraries/System.Private.Xml/src/System/Xml/XmlResolver.ThrowingResolver.cs @@ -8,6 +8,7 @@ namespace System.Xml { public abstract partial class XmlResolver { +#pragma warning disable PLATDOC003 // Not platform-specific; property lives in this file by convention /// /// Gets an XML resolver which forbids entity resolution. /// @@ -21,6 +22,7 @@ public abstract partial class XmlResolver /// prohibited, even when DTD processing is otherwise enabled. /// public static XmlResolver ThrowingResolver => XmlThrowingResolver.s_singleton; +#pragma warning restore PLATDOC003 // An XmlResolver that forbids all external entity resolution. private sealed class XmlThrowingResolver : XmlResolver