diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index 258fc7e..8511595 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -21,8 +21,10 @@ concurrency: group: scorecard-${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true -# Read-only by default; the single job widens only the two write scopes it needs. -permissions: read-all +# Minimal read at the workflow level; the single job overrides with only the two write scopes it +# needs. Enumerated rather than `read-all` so the default token grants exactly one explicit scope. +permissions: + contents: read jobs: analysis: diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml index 2520f44..7ec58f7 100644 --- a/.github/workflows/sonar.yml +++ b/.github/workflows/sonar.yml @@ -66,6 +66,9 @@ jobs: # The build must sit between `begin` and `end`: the scanner hooks MSBuild to # observe the compilation, so it cannot use a pre-built or --no-build output. + # *.verified.* are Verify snapshot oracles — generated test fixtures, not production source. Excluding them + # keeps Sonar from analysing the emitted HTML/JS/Markdown as if it were hand-written code (and from counting + # the near-identical snapshots as duplication). - name: Begin analysis env: SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} @@ -76,6 +79,7 @@ jobs: /d:sonar.token="$SONAR_TOKEN" /d:sonar.host.url="https://sonarcloud.io" /d:sonar.cs.opencover.reportsPaths="artifacts/coverage/**/coverage.opencover.xml" + /d:sonar.exclusions="**/*.verified.*" - name: Build # Disable the CI warning ratchet (Directory.Build.props) for the analysis build only. The scanner diff --git a/Directory.Packages.props b/Directory.Packages.props index 91bb243..fcad955 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -24,6 +24,7 @@ --> + diff --git a/FirstClassErrors.Analyzers/Descriptors.cs b/FirstClassErrors.Analyzers/Descriptors.cs index 78fb9c6..9d8fd35 100644 --- a/FirstClassErrors.Analyzers/Descriptors.cs +++ b/FirstClassErrors.Analyzers/Descriptors.cs @@ -17,7 +17,7 @@ internal static class Descriptors { isEnabledByDefault: true, description: "An ErrorCode is compared by value, so the same literal code created more than once yields equal instances that documentation extraction and lookups collapse into a single identity. Detection is per-compilation and limited to literal codes.", helpLinkUri: HelpLinks.For(DiagnosticIds.DuplicateErrorCode), - customTags: new[] { WellKnownDiagnosticTags.CompilationEnd }); + WellKnownDiagnosticTags.CompilationEnd); public static readonly DiagnosticDescriptor EmptyErrorCode = new( id: DiagnosticIds.EmptyErrorCode, @@ -138,7 +138,7 @@ internal static class Descriptors { isEnabledByDefault: true, description: "Documentation extraction groups by error code and keeps a single entry per code. Two documented factories that share the same code field silently collapse to one in the catalog.", helpLinkUri: HelpLinks.For(DiagnosticIds.DuplicateDocumentedCode), - customTags: new[] { WellKnownDiagnosticTags.CompilationEnd }); + WellKnownDiagnosticTags.CompilationEnd); public static readonly DiagnosticDescriptor ExampleDoesNotCallDocumentedFactory = new( id: DiagnosticIds.ExampleDoesNotCallDocumentedFactory, diff --git a/FirstClassErrors.Analyzers/DuplicateDocumentedCodeAnalyzer.cs b/FirstClassErrors.Analyzers/DuplicateDocumentedCodeAnalyzer.cs index 5bc4139..54acda4 100644 --- a/FirstClassErrors.Analyzers/DuplicateDocumentedCodeAnalyzer.cs +++ b/FirstClassErrors.Analyzers/DuplicateDocumentedCodeAnalyzer.cs @@ -47,21 +47,35 @@ private static void Collect( if (context.OwningSymbol is not IMethodSymbol method) { return; } if (!SymbolFacts.HasAttribute(method, symbols.DocumentedByAttribute!)) { return; } - foreach (IOperation block in context.OperationBlocks) { - foreach (IOperation operation in OperationFacts.EnumerateOperations(block)) { - if (operation is not IInvocationOperation invocation) { continue; } - if (invocation.TargetMethod.Name != CreateMethodName) { continue; } - if (!SymbolFacts.IsOrInheritsFrom(invocation.TargetMethod.ContainingType, symbols.Error!)) { continue; } - if (invocation.Arguments.Length == 0) { continue; } + // The outermost error-factory Create identifies the produced code. + IInvocationOperation? create = FindErrorFactoryCreate(context.OperationBlocks, symbols); + if (create is null) { return; } - if (TryGetCodeField(invocation.Arguments[0].Value, out ISymbol? codeField)) { - usagesByCodeField.GetOrAdd(codeField!, _ => new ConcurrentBag()) - .Add(method.Locations.FirstOrDefault() ?? Location.None); - } + if (TryGetCodeField(create.Arguments[0].Value, out ISymbol? codeField)) { + usagesByCodeField.GetOrAdd(codeField!, _ => new ConcurrentBag()) + .Add(method.Locations.FirstOrDefault() ?? Location.None); + } + } - return; // the outermost error-factory Create identifies the produced code + /// Finds the first Error.Create(...) invocation carrying at least one argument, across the blocks. + private static IInvocationOperation? FindErrorFactoryCreate(ImmutableArray blocks, KnownSymbols symbols) { + foreach (IOperation block in blocks) { + foreach (IOperation operation in OperationFacts.EnumerateOperations(block)) { + IInvocationOperation? create = AsErrorFactoryCreate(operation, symbols); + if (create is not null) { return create; } } } + + return null; + } + + /// Returns as an Error.Create(...) invocation with arguments, or null. + private static IInvocationOperation? AsErrorFactoryCreate(IOperation operation, KnownSymbols symbols) { + if (operation is not IInvocationOperation invocation) { return null; } + if (invocation.TargetMethod.Name != CreateMethodName) { return null; } + if (!SymbolFacts.IsOrInheritsFrom(invocation.TargetMethod.ContainingType, symbols.Error!)) { return null; } + + return invocation.Arguments.Length == 0 ? null : invocation; } private static bool TryGetCodeField(IOperation codeArgument, out ISymbol? codeField) { diff --git a/FirstClassErrors.Analyzers/ExampleDoesNotCallDocumentedFactoryAnalyzer.cs b/FirstClassErrors.Analyzers/ExampleDoesNotCallDocumentedFactoryAnalyzer.cs index a961356..c10bd51 100644 --- a/FirstClassErrors.Analyzers/ExampleDoesNotCallDocumentedFactoryAnalyzer.cs +++ b/FirstClassErrors.Analyzers/ExampleDoesNotCallDocumentedFactoryAnalyzer.cs @@ -46,10 +46,8 @@ private static void Analyze(OperationAnalysisContext context, INamedTypeSymbol e INamedTypeSymbol? documentingType = context.ContainingSymbol.ContainingType; if (documentingType is null) { return; } - foreach (IOperation example in GetExampleOperations(invocation)) { - if (!ExampleInvokesMemberOf(example, documentingType)) { - context.ReportDiagnostic(Diagnostic.Create(Descriptors.ExampleDoesNotCallDocumentedFactory, example.Syntax.GetLocation(), documentingType.Name)); - } + foreach (IOperation example in GetExampleOperations(invocation).Where(example => !ExampleInvokesMemberOf(example, documentingType))) { + context.ReportDiagnostic(Diagnostic.Create(Descriptors.ExampleDoesNotCallDocumentedFactory, example.Syntax.GetLocation(), documentingType.Name)); } } diff --git a/FirstClassErrors.Analyzers/SymbolFacts.cs b/FirstClassErrors.Analyzers/SymbolFacts.cs index f7df652..c393e2a 100644 --- a/FirstClassErrors.Analyzers/SymbolFacts.cs +++ b/FirstClassErrors.Analyzers/SymbolFacts.cs @@ -17,15 +17,15 @@ public static bool TryGetDocumentedBy( out AttributeData? attribute, out string? targetMethodName) { - foreach (AttributeData candidate in method.GetAttributes()) { - if (SymbolEqualityComparer.Default.Equals(candidate.AttributeClass, documentedByAttributeType)) { - attribute = candidate; - targetMethodName = candidate.ConstructorArguments.Length == 1 - ? candidate.ConstructorArguments[0].Value as string - : null; - - return true; - } + AttributeData? candidate = method.GetAttributes() + .FirstOrDefault(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, documentedByAttributeType)); + if (candidate is not null) { + attribute = candidate; + targetMethodName = candidate.ConstructorArguments.Length == 1 + ? candidate.ConstructorArguments[0].Value as string + : null; + + return true; } attribute = null; @@ -35,13 +35,7 @@ public static bool TryGetDocumentedBy( } public static bool HasAttribute(ISymbol symbol, INamedTypeSymbol attributeType) { - foreach (AttributeData attribute in symbol.GetAttributes()) { - if (SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, attributeType)) { - return true; - } - } - - return false; + return symbol.GetAttributes().Any(attribute => SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, attributeType)); } public static bool IsOrInheritsFrom(ITypeSymbol type, INamedTypeSymbol target) { diff --git a/FirstClassErrors.Cli.UnitTests/ConfigurationStoreTests.cs b/FirstClassErrors.Cli.UnitTests/ConfigurationStoreTests.cs new file mode 100644 index 0000000..2bb3f50 --- /dev/null +++ b/FirstClassErrors.Cli.UnitTests/ConfigurationStoreTests.cs @@ -0,0 +1,45 @@ +#region Usings declarations + +using JetBrains.Annotations; + +using NFluent; + +#endregion + +namespace FirstClassErrors.Cli.UnitTests; + +[TestSubject(typeof(ConfigurationStore))] +public sealed class ConfigurationStoreTests { + + [Fact(DisplayName = "Loading a configuration from a missing file returns an empty configuration.")] + public void LoadingAMissingFileReturnsAnEmptyConfiguration() { + // Setup: a path guaranteed not to exist. + string path = CliTestHelpers.NonExistentConfigPath(); + + // Exercise + CliConfiguration configuration = ConfigurationStore.Load(path); + + // Verify: a fresh configuration carries none of the persisted defaults. + Check.That(configuration.Solution).IsNull(); + Check.That(configuration.Format).IsNull(); + Check.That(configuration.NoBuild).IsNull(); + } + + [Fact(DisplayName = "Loading a configuration from a blank file returns an empty configuration.")] + public void LoadingABlankFileReturnsAnEmptyConfiguration() { + // Setup: an existing but blank file exercises the whitespace short-circuit rather than the missing-file one. + string path = Path.Combine(Path.GetTempPath(), $"fce-blank-config-{Guid.NewGuid():N}.json"); + File.WriteAllText(path, " \n"); + + try { + // Exercise + CliConfiguration configuration = ConfigurationStore.Load(path); + + // Verify + Check.That(configuration.Solution).IsNull(); + } finally { + File.Delete(path); + } + } + +} diff --git a/FirstClassErrors.Cli.UnitTests/ConsoleAndFileOutputSinkTests.cs b/FirstClassErrors.Cli.UnitTests/ConsoleAndFileOutputSinkTests.cs new file mode 100644 index 0000000..c6bf28e --- /dev/null +++ b/FirstClassErrors.Cli.UnitTests/ConsoleAndFileOutputSinkTests.cs @@ -0,0 +1,33 @@ +#region Usings declarations + +using JetBrains.Annotations; + +using NFluent; + +#endregion + +namespace FirstClassErrors.Cli.UnitTests; + +[TestSubject(typeof(ConsoleAndFileOutputSink))] +public sealed class ConsoleAndFileOutputSinkTests { + + [Fact(DisplayName = "Writing a file into a not-yet-existing nested directory creates the directory and the file.")] + public void WritingIntoANestedDirectoryCreatesItAndTheFile() { + // Setup: a target whose parent directory does not exist yet, so WriteFile must create it. + ConsoleAndFileOutputSink sink = new(); + string rootDir = Path.Combine(Path.GetTempPath(), $"fce-sink-{Guid.NewGuid():N}"); + string filePath = Path.Combine(rootDir, "nested", "out.txt"); + + try { + // Exercise + sink.WriteFile(filePath, "content"); + + // Verify: the nested directory was created and the content written. + Check.That(File.Exists(filePath)).IsTrue(); + Check.That(File.ReadAllText(filePath)).IsEqualTo("content"); + } finally { + if (Directory.Exists(rootDir)) { Directory.Delete(rootDir, recursive: true); } + } + } + +} diff --git a/FirstClassErrors.Cli/ConfigShowCommand.cs b/FirstClassErrors.Cli/ConfigShowCommand.cs index ba119e6..d734ea5 100644 --- a/FirstClassErrors.Cli/ConfigShowCommand.cs +++ b/FirstClassErrors.Cli/ConfigShowCommand.cs @@ -12,7 +12,7 @@ internal sealed class ConfigShowCommand : Command { protected override int Execute(CommandContext context, ConfigScopedSettings settings, CancellationToken cancellationToken) { string path = ConfigurationStore.Resolve(settings.ConfigPath); - if (ConfigurationStore.Exists(path) is false) { + if (!ConfigurationStore.Exists(path)) { Console.Out.WriteLine($"No configuration at '{path}'. Run 'fce config init' to create one."); return 0; diff --git a/FirstClassErrors.Cli/ConfigurationStore.cs b/FirstClassErrors.Cli/ConfigurationStore.cs index 55817f6..d22278c 100644 --- a/FirstClassErrors.Cli/ConfigurationStore.cs +++ b/FirstClassErrors.Cli/ConfigurationStore.cs @@ -38,7 +38,7 @@ public static bool Exists(string path) { /// Loads the configuration, returning an empty one when the file is missing or blank. public static CliConfiguration Load(string path) { - if (File.Exists(path) is false) { return new CliConfiguration(); } + if (!File.Exists(path)) { return new CliConfiguration(); } string json = File.ReadAllText(path); if (string.IsNullOrWhiteSpace(json)) { return new CliConfiguration(); } @@ -58,7 +58,7 @@ public static CliConfiguration Load(string path) { /// public static void Save(string path, CliConfiguration configuration) { string? directory = Path.GetDirectoryName(path); - if (string.IsNullOrEmpty(directory) is false) { Directory.CreateDirectory(directory); } + if (!string.IsNullOrEmpty(directory)) { Directory.CreateDirectory(directory); } string json = JsonSerializer.Serialize(configuration, SerializerOptions); string tempPath = path + ".tmp"; diff --git a/FirstClassErrors.Cli/ConsoleAndFileOutputSink.cs b/FirstClassErrors.Cli/ConsoleAndFileOutputSink.cs index 6ab2bb1..326f45a 100644 --- a/FirstClassErrors.Cli/ConsoleAndFileOutputSink.cs +++ b/FirstClassErrors.Cli/ConsoleAndFileOutputSink.cs @@ -13,7 +13,7 @@ public void WriteStandardOutput(string content) { public void WriteFile(string fullPath, string content) { string? directory = Path.GetDirectoryName(fullPath); - if (string.IsNullOrEmpty(directory) is false) { + if (!string.IsNullOrEmpty(directory)) { Directory.CreateDirectory(directory); } diff --git a/FirstClassErrors.Cli/DocumentationOutputWriter.cs b/FirstClassErrors.Cli/DocumentationOutputWriter.cs index 30d84d3..660fd7b 100644 --- a/FirstClassErrors.Cli/DocumentationOutputWriter.cs +++ b/FirstClassErrors.Cli/DocumentationOutputWriter.cs @@ -36,10 +36,10 @@ public void Write(IReadOnlyList documents, string format, stri throw new InvalidOperationException($"The '{format}' renderer produced no documents; a renderer must return at least one document."); } - bool hasOutput = string.IsNullOrWhiteSpace(outputPath) is false; + bool hasOutput = !string.IsNullOrWhiteSpace(outputPath); // No target: only a single document can go to standard output. - if (hasOutput is false) { + if (!hasOutput) { if (documents.Count > 1) { throw new InvalidOperationException("This layout produces several files; specify an output directory with --output (or 'output' in the configuration)."); } @@ -54,7 +54,7 @@ public void Write(IReadOnlyList documents, string format, stri // Treat the target as a directory when there are several files, when it already exists as one, or when the // path ends with a separator. Otherwise a single document is written to the given file path verbatim. bool asDirectory = documents.Count > 1 || Directory.Exists(fullOutput) || EndsWithSeparator(outputPath!); - if (asDirectory is false) { + if (!asDirectory) { _sink.WriteFile(fullOutput, documents[0].Content); logger.Info($"Documentation written to '{fullOutput}'."); @@ -86,7 +86,7 @@ internal static string ResolveWithinOutput(string outputDirectory, string relati ? outputDirectory : outputDirectory + Path.DirectorySeparatorChar; - if (target.StartsWith(root, StringComparison.Ordinal) is false) { + if (!target.StartsWith(root, StringComparison.Ordinal)) { throw new InvalidOperationException( $"The renderer produced a document whose path '{relativePath}' escapes the output directory '{outputDirectory}'."); } diff --git a/FirstClassErrors.Cli/GenerateCommand.cs b/FirstClassErrors.Cli/GenerateCommand.cs index da93b35..1c45022 100644 --- a/FirstClassErrors.Cli/GenerateCommand.cs +++ b/FirstClassErrors.Cli/GenerateCommand.cs @@ -68,7 +68,7 @@ internal int Run(GenerateSettings settings, CancellationToken cancellationToken) return 1; } - if (resolved.HasSolution is false && resolved.HasAssemblies is false) { + if (!resolved.HasSolution && !resolved.HasAssemblies) { logger.Error("No source: pass --solution/--assemblies, or set 'solution'/'assemblies' in the configuration."); return 1; @@ -84,7 +84,7 @@ internal int Run(GenerateSettings settings, CancellationToken cancellationToken) IReadOnlyList customRenderers = RendererLoader.Load(configuration.Renderers, configDir, logger); IErrorDocumentationRenderer renderer = RendererCatalog.Create(resolved.Format, customRenderers); - if (renderer.SupportedLayouts.Contains(resolved.Layout, StringComparer.OrdinalIgnoreCase) is false) { + if (!renderer.SupportedLayouts.Contains(resolved.Layout, StringComparer.OrdinalIgnoreCase)) { logger.Error($"The '{resolved.Format}' format does not support the '{resolved.Layout}' layout. Supported layouts: {string.Join(", ", renderer.SupportedLayouts)}."); return 1; @@ -100,7 +100,7 @@ internal int Run(GenerateSettings settings, CancellationToken cancellationToken) } SolutionGenerationOptions options = new() { - BuildSolution = resolved.NoBuild is false, + BuildSolution = !resolved.NoBuild, Configuration = resolved.BuildConfiguration, TargetFramework = resolved.Framework, FailureBehavior = resolved.Strict ? FailureBehavior.Stop : FailureBehavior.Continue, diff --git a/FirstClassErrors.Cli/GenerateOptionsResolver.cs b/FirstClassErrors.Cli/GenerateOptionsResolver.cs index cf9cf31..3fcf276 100644 --- a/FirstClassErrors.Cli/GenerateOptionsResolver.cs +++ b/FirstClassErrors.Cli/GenerateOptionsResolver.cs @@ -36,7 +36,7 @@ public static ResolvedGenerateOptions Resolve(GenerateSettings settings, CliConf // combine with a configured 'solution'). string? solution; string[] assemblies; - if (string.IsNullOrWhiteSpace(settings.SolutionPath) is false || settings.AssemblyPaths.Length > 0) { + if (!string.IsNullOrWhiteSpace(settings.SolutionPath) || settings.AssemblyPaths.Length > 0) { solution = settings.SolutionPath; assemblies = settings.AssemblyPaths; } else { @@ -47,7 +47,7 @@ public static ResolvedGenerateOptions Resolve(GenerateSettings settings, CliConf return new ResolvedGenerateOptions( Solution: solution, Assemblies: assemblies, - HasSolution: string.IsNullOrWhiteSpace(solution) is false, + HasSolution: !string.IsNullOrWhiteSpace(solution), HasAssemblies: assemblies.Length > 0, Format: NormalizeFormat(FirstNonEmpty(settings.Format, configuration.Format) ?? "json"), Layout: (FirstNonEmpty(settings.Layout, configuration.Layout) ?? "single").Trim().ToLowerInvariant(), @@ -62,8 +62,8 @@ public static ResolvedGenerateOptions Resolve(GenerateSettings settings, CliConf } internal static string? FirstNonEmpty(string? primary, string? fallback) { - if (string.IsNullOrWhiteSpace(primary) is false) { return primary; } - if (string.IsNullOrWhiteSpace(fallback) is false) { return fallback; } + if (!string.IsNullOrWhiteSpace(primary)) { return primary; } + if (!string.IsNullOrWhiteSpace(fallback)) { return fallback; } return null; } diff --git a/FirstClassErrors.Cli/InitCommand.cs b/FirstClassErrors.Cli/InitCommand.cs index 3d801ec..2023466 100644 --- a/FirstClassErrors.Cli/InitCommand.cs +++ b/FirstClassErrors.Cli/InitCommand.cs @@ -25,7 +25,7 @@ internal sealed class InitCommand : Command { protected override int Execute(CommandContext context, InitSettings settings, CancellationToken cancellationToken) { string path = ConfigurationStore.Resolve(settings.ConfigPath); - if (ConfigurationStore.Exists(path) && settings.Force is false) { + if (ConfigurationStore.Exists(path) && !settings.Force) { Console.Error.WriteLine($"error: a configuration already exists at '{path}'. Use --force to overwrite."); return 1; diff --git a/FirstClassErrors.Cli/Program.cs b/FirstClassErrors.Cli/Program.cs index 8df83b4..2ac5949 100644 --- a/FirstClassErrors.Cli/Program.cs +++ b/FirstClassErrors.Cli/Program.cs @@ -30,4 +30,4 @@ // Spectre handles argument parsing, validation errors and --help. Runtime failures are handled inside each command // so the tool reports them as a terse "error: …" line rather than a stack trace. -return app.Run(args); +return await app.RunAsync(args); diff --git a/FirstClassErrors.Cli/RendererAddCommand.cs b/FirstClassErrors.Cli/RendererAddCommand.cs index 76187e7..eec95c2 100644 --- a/FirstClassErrors.Cli/RendererAddCommand.cs +++ b/FirstClassErrors.Cli/RendererAddCommand.cs @@ -1,6 +1,7 @@ #region Usings declarations using System.ComponentModel; +using System.Diagnostics.CodeAnalysis; using System.Reflection; using FirstClassErrors.GenDoc.Rendering; @@ -26,12 +27,16 @@ internal sealed class RendererReferenceSettings : ConfigScopedSettings { /// internal sealed class RendererAddCommand : Command { + [SuppressMessage("Major Code Smell", "S3885:\"Assembly.Load\" should be used instead of \"Assembly.LoadFrom\"", + Justification = + "The renderer library is validated by loading it from its file path; Assembly.Load resolves by name, " + + "not path. LoadFrom is deliberate — it probes the library's own directory for its dependencies.")] protected override int Execute(CommandContext context, RendererReferenceSettings settings, CancellationToken cancellationToken) { string path = ConfigurationStore.Resolve(settings.ConfigPath); string configDir = Path.GetDirectoryName(path) ?? Directory.GetCurrentDirectory(); string library = RendererLoader.Resolve(settings.LibraryPath, configDir); - if (File.Exists(library) is false) { + if (!File.Exists(library)) { Console.Error.WriteLine($"error: renderer library not found: '{library}'."); return 1; diff --git a/FirstClassErrors.Cli/RendererCatalog.cs b/FirstClassErrors.Cli/RendererCatalog.cs index 497d07e..d6317f3 100644 --- a/FirstClassErrors.Cli/RendererCatalog.cs +++ b/FirstClassErrors.Cli/RendererCatalog.cs @@ -23,10 +23,11 @@ internal static class RendererCatalog { () => new HtmlErrorDocumentationRenderer() ]; + /// The built-in format identifiers, computed once (the built-in renderers never change at runtime). + private static readonly IReadOnlyList BuiltInFormatList = BuiltInFactories.Select(factory => factory().Format).ToList(); + /// Gets the built-in format identifiers, as declared by the built-in renderers. - public static IReadOnlyList BuiltInFormats { - get { return BuiltInFactories.Select(factory => factory().Format).ToList(); } - } + public static IReadOnlyList BuiltInFormats => BuiltInFormatList; /// /// Creates the renderer whose declared format matches , preferring a built-in over @@ -43,10 +44,10 @@ public static IErrorDocumentationRenderer Create(string format, IReadOnlyList string.Equals(renderer.Format, format, StringComparison.OrdinalIgnoreCase)); + if (customRenderer is not null) { + return customRenderer; } IEnumerable available = BuiltInFormats.Concat(customRenderers.Select(renderer => renderer.Format)) diff --git a/FirstClassErrors.Cli/RendererListCommand.cs b/FirstClassErrors.Cli/RendererListCommand.cs index 49b941e..f9a2f66 100644 --- a/FirstClassErrors.Cli/RendererListCommand.cs +++ b/FirstClassErrors.Cli/RendererListCommand.cs @@ -1,5 +1,6 @@ #region Usings declarations +using System.Diagnostics.CodeAnalysis; using System.Reflection; using FirstClassErrors.GenDoc.Rendering; @@ -16,6 +17,10 @@ namespace FirstClassErrors.Cli; /// internal sealed class RendererListCommand : Command { + [SuppressMessage("Major Code Smell", "S3885:\"Assembly.Load\" should be used instead of \"Assembly.LoadFrom\"", + Justification = + "Each configured library is inspected by loading it from its file path; Assembly.Load resolves by " + + "name, not path. LoadFrom is deliberate — it probes the library's own directory for its dependencies.")] protected override int Execute(CommandContext context, ConfigScopedSettings settings, CancellationToken cancellationToken) { Console.Out.WriteLine("Built-in formats:"); foreach (string format in RendererCatalog.BuiltInFormats) { @@ -36,7 +41,7 @@ protected override int Execute(CommandContext context, ConfigScopedSettings sett Console.Out.WriteLine($"Custom renderers ({path}):"); foreach (string reference in configuration.Renderers) { string library = RendererLoader.Resolve(reference, configDir); - if (File.Exists(library) is false) { + if (!File.Exists(library)) { Console.Out.WriteLine($" - {reference} (missing: {library})"); continue; diff --git a/FirstClassErrors.Cli/RendererLoader.cs b/FirstClassErrors.Cli/RendererLoader.cs index ae2e64a..7998a61 100644 --- a/FirstClassErrors.Cli/RendererLoader.cs +++ b/FirstClassErrors.Cli/RendererLoader.cs @@ -1,5 +1,6 @@ #region Usings declarations +using System.Diagnostics.CodeAnalysis; using System.Reflection; using FirstClassErrors.GenDoc; @@ -32,12 +33,17 @@ public static string Resolve(string configuredPath, string baseDirectory) { /// Loads every renderer referenced by . Problems (missing file, load error, /// no renderer inside) are logged as warnings and skipped rather than aborting the run. /// + [SuppressMessage("Major Code Smell", "S3885:\"Assembly.Load\" should be used instead of \"Assembly.LoadFrom\"", + Justification = + "A renderer plugin is loaded from a file path, so Assembly.Load — which resolves by assembly name, " + + "not by path — cannot be used. LoadFrom is deliberate: it probes the plugin's own directory for that " + + "plugin's dependencies, which loading into the default context (LoadFromAssemblyPath) would not.")] public static IReadOnlyList Load(IReadOnlyList assemblyPaths, string baseDirectory, IGenerationLogger logger) { List renderers = []; foreach (string configured in assemblyPaths) { string path = Resolve(configured, baseDirectory); - if (File.Exists(path) is false) { + if (!File.Exists(path)) { logger.Warning($"Configured renderer library not found: '{path}'."); continue; @@ -73,7 +79,7 @@ public static IReadOnlyList InstantiateRenderers(As List renderers = []; foreach (Type? type in types) { if (type is null || type.IsAbstract || type.IsInterface) { continue; } - if (typeof(IErrorDocumentationRenderer).IsAssignableFrom(type) is false) { continue; } + if (!typeof(IErrorDocumentationRenderer).IsAssignableFrom(type)) { continue; } if (type.GetConstructor(Type.EmptyTypes) is null) { continue; } if (Activator.CreateInstance(type) is IErrorDocumentationRenderer renderer) { renderers.Add(renderer); } diff --git a/FirstClassErrors.Cli/RendererRemoveCommand.cs b/FirstClassErrors.Cli/RendererRemoveCommand.cs index c24df86..9f20408 100644 --- a/FirstClassErrors.Cli/RendererRemoveCommand.cs +++ b/FirstClassErrors.Cli/RendererRemoveCommand.cs @@ -15,7 +15,7 @@ internal sealed class RendererRemoveCommand : Command protected override int Execute(CommandContext context, RendererReferenceSettings settings, CancellationToken cancellationToken) { string path = ConfigurationStore.Resolve(settings.ConfigPath); - if (ConfigurationStore.Exists(path) is false) { + if (!ConfigurationStore.Exists(path)) { Console.Error.WriteLine($"error: no configuration at '{path}'. Run 'fce init' first."); return 1; diff --git a/FirstClassErrors.GenDoc.UnitTests/FirstClassErrors.GenDoc.UnitTests.csproj b/FirstClassErrors.GenDoc.UnitTests/FirstClassErrors.GenDoc.UnitTests.csproj index 0aeccb3..1cba308 100644 --- a/FirstClassErrors.GenDoc.UnitTests/FirstClassErrors.GenDoc.UnitTests.csproj +++ b/FirstClassErrors.GenDoc.UnitTests/FirstClassErrors.GenDoc.UnitTests.csproj @@ -8,6 +8,7 @@ + all runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/FirstClassErrors.GenDoc.UnitTests/GeneratedHtmlWellFormednessTests.cs b/FirstClassErrors.GenDoc.UnitTests/GeneratedHtmlWellFormednessTests.cs new file mode 100644 index 0000000..a2527dc --- /dev/null +++ b/FirstClassErrors.GenDoc.UnitTests/GeneratedHtmlWellFormednessTests.cs @@ -0,0 +1,98 @@ +#region Usings declarations + +using System.Globalization; + +using AngleSharp.Html.Parser; + +using FirstClassErrors.GenDoc.Rendering; +using FirstClassErrors.Usage.Model; + +using NFluent; + +#endregion + +namespace FirstClassErrors.GenDoc.UnitTests; + +/// +/// Well-formedness gate for the HTML the documentation generator emits. The snapshot tests only lock the output +/// byte-for-byte against an approved oracle, so a stably-malformed page would still pass them. These tests instead +/// parse every generated page with a strict HTML5 parser, so a regression that makes the generator emit invalid +/// markup — an unescaped <, a broken attribute, a misnested or unclosed tag — fails the build rather than +/// being silently baked into a new snapshot. +/// +/// +/// Strict parsing has teeth: it throws on exactly those malformations (verified against unescaped <, +/// unterminated tags, misnesting, stray end tags and duplicate attributes). It is not over-strict either — every +/// document the generator currently emits parses cleanly. The emitted CSS/JS live inside the page, so this also +/// guards the <style>/<script> islands, which SonarJS cannot see from the C# string +/// constants that hold them. +/// +public sealed class GeneratedHtmlWellFormednessTests { + + #region Statics members declarations + + private const string SampleService = "sample-service"; + + private static ErrorDocumentationExtractionResult ExtractFor(CultureInfo culture) { + CultureInfo previous = CultureInfo.CurrentUICulture; + CultureInfo.CurrentUICulture = culture; + try { + return AssemblyErrorDocumentationReader.GetErrorDocumentationFrom(typeof(Temperature).Assembly); + } finally { + CultureInfo.CurrentUICulture = previous; + } + } + + // A fresh parser per call keeps the check safe under xUnit's parallelism (HtmlParser is not meant to be shared + // across concurrent parses). Strict mode turns any HTML5 parse error into an HtmlParseException. + private static void CheckWellFormed(string label, string html) { + HtmlParser parser = new(new HtmlParserOptions { IsStrictMode = true }); + Check.ThatCode(() => parser.ParseDocument(html)) + .As($"generated HTML document '{label}'") + .DoesNotThrow(); + } + + #endregion + + [Fact(DisplayName = "The single-page HTML rendering of the Usage catalog is well-formed.")] + public void TheSinglePageHtmlIsWellFormed() { + string html = new HtmlErrorDocumentationRenderer() + .Render(ExtractFor(CultureInfo.InvariantCulture).Documentation, new RenderRequest(RenderLayouts.Single, CultureInfo.InvariantCulture, SampleService))[0] + .Content; + + CheckWellFormed("single", html); + } + + [Fact(DisplayName = "Every HTML page of the split rendering of the Usage catalog is well-formed.")] + public void EachSplitHtmlPageIsWellFormed() { + IReadOnlyList documents = + new HtmlErrorDocumentationRenderer().Render(ExtractFor(CultureInfo.InvariantCulture).Documentation, new RenderRequest(RenderLayouts.Split, CultureInfo.InvariantCulture, SampleService)); + + List htmlPages = documents + .Where(document => document.RelativePath.EndsWith(".html", StringComparison.Ordinal)) + .ToList(); + + // Guard against a vacuous pass: the split layout must produce the index plus one page per error. + Check.That(htmlPages).Not.IsEmpty(); + + foreach (RenderedDocument page in htmlPages) { + CheckWellFormed(page.RelativePath, page.Content); + } + } + + [Theory(DisplayName = "The localized HTML rendering of the Usage catalog is well-formed per language.")] + [InlineData("fr")] + [InlineData("es")] + [InlineData("de")] + [InlineData("sv")] + public void TheLocalizedHtmlIsWellFormed(string culture) { + CultureInfo cultureInfo = CultureInfo.GetCultureInfo(culture); + + string html = new HtmlErrorDocumentationRenderer() + .Render(ExtractFor(cultureInfo).Documentation, new RenderRequest(RenderLayouts.Single, cultureInfo, SampleService))[0] + .Content; + + CheckWellFormed($"single:{culture}", html); + } + +} diff --git a/FirstClassErrors.GenDoc.UnitTests/SolutionErrorDocumentationGeneratorTests.cs b/FirstClassErrors.GenDoc.UnitTests/SolutionErrorDocumentationGeneratorTests.cs index 6cbbc64..e6a89db 100644 --- a/FirstClassErrors.GenDoc.UnitTests/SolutionErrorDocumentationGeneratorTests.cs +++ b/FirstClassErrors.GenDoc.UnitTests/SolutionErrorDocumentationGeneratorTests.cs @@ -11,6 +11,10 @@ namespace FirstClassErrors.GenDoc.UnitTests; [TestSubject(typeof(SolutionErrorDocumentationGenerator))] public sealed class SolutionErrorDocumentationGeneratorTests { + // Held as static readonly fields rather than inline new[] {…} arguments so the arrays are allocated once (CA1861). + private static readonly string[] AppAssemblyPaths = ["app.dll"]; + private static readonly string[] MissingAssemblyPaths = ["this-assembly-does-not-exist.dll"]; + [Fact(DisplayName = "GetErrorDocumentationFrom rejects a null solution path.")] public void GetErrorDocumentationFromRejectsANullSolutionPath() { // Exercise & verify @@ -81,7 +85,7 @@ public void GetErrorDocumentationFromAssembliesRejectsANullPathList() { [Fact(DisplayName = "GetErrorDocumentationFromAssemblies rejects null options.")] public void GetErrorDocumentationFromAssembliesRejectsNullOptions() { // Exercise & verify - Check.ThatCode(() => SolutionErrorDocumentationGenerator.GetErrorDocumentationFromAssemblies(new[] { "app.dll" }, null!)) + Check.ThatCode(() => SolutionErrorDocumentationGenerator.GetErrorDocumentationFromAssemblies(AppAssemblyPaths, null!)) .Throws(); } @@ -92,7 +96,7 @@ public void AMissingAssemblyIsSkippedWhenContinuing() { // Exercise IEnumerable result = - SolutionErrorDocumentationGenerator.GetErrorDocumentationFromAssemblies(new[] { "this-assembly-does-not-exist.dll" }, options); + SolutionErrorDocumentationGenerator.GetErrorDocumentationFromAssemblies(MissingAssemblyPaths, options); // Verify: no worker is launched because no assembly resolved; the result is empty. Check.That(result).IsEmpty(); @@ -108,7 +112,7 @@ public void ASkippedAssemblyIsLoggedWhenContinuing() { }; // Exercise - SolutionErrorDocumentationGenerator.GetErrorDocumentationFromAssemblies(new[] { "this-assembly-does-not-exist.dll" }, options); + SolutionErrorDocumentationGenerator.GetErrorDocumentationFromAssemblies(MissingAssemblyPaths, options); // Verify: the skipped assembly leaves a warning trace mentioning the missing file. Check.That(logger.Warnings).HasSize(1); @@ -132,7 +136,7 @@ public void AMissingAssemblyAbortsWhenStopping() { SolutionGenerationOptions options = new() { FailureBehavior = FailureBehavior.Stop }; // Exercise & verify - Check.ThatCode(() => SolutionErrorDocumentationGenerator.GetErrorDocumentationFromAssemblies(new[] { "this-assembly-does-not-exist.dll" }, options)) + Check.ThatCode(() => SolutionErrorDocumentationGenerator.GetErrorDocumentationFromAssemblies(MissingAssemblyPaths, options)) .Throws(); } diff --git a/FirstClassErrors.GenDoc.UnitTests/UsageDocumentationSnapshotTests.TheLocalizedHtmlRenderingOfTheUsageCatalog_culture=de.verified.html b/FirstClassErrors.GenDoc.UnitTests/UsageDocumentationSnapshotTests.TheLocalizedHtmlRenderingOfTheUsageCatalog_culture=de.verified.html index b78f413..c7678f4 100644 --- a/FirstClassErrors.GenDoc.UnitTests/UsageDocumentationSnapshotTests.TheLocalizedHtmlRenderingOfTheUsageCatalog_culture=de.verified.html +++ b/FirstClassErrors.GenDoc.UnitTests/UsageDocumentationSnapshotTests.TheLocalizedHtmlRenderingOfTheUsageCatalog_culture=de.verified.html @@ -81,7 +81,7 @@ .group-title { font-size: 1.3rem; } } - + @@ -540,14 +540,24 @@

TEMPERATURE_BELOW_ABSOLUTE_ZERO Saltar al contenido @@ -540,14 +540,24 @@

TEMPERATURE_BELOW_ABSOLUTE_ZERO Aller au contenu @@ -540,14 +540,24 @@

TEMPERATURE_BELOW_ABSOLUTE_ZERO Hoppa till innehåll @@ -540,14 +540,24 @@

TEMPERATURE_BELOW_ABSOLUTE_ZERO Skip to content @@ -540,14 +540,24 @@

TEMPERATURE_BELOW_ABSOLUTE_ZERO Skip to content @@ -128,14 +128,24 @@

AMOUNT_CURRENCY_MISMATCH AMOUNT_CURRENCY_MISMATCH - + @@ -140,14 +140,24 @@

BANK_TRANSACTION_FILE_DATE_OUT_OF_STATEMENT_PERIOD (function () { var KEY = 'fce-theme'; var root = document.documentElement; - function apply(t) { if (t === 'light' || t === 'dark') { root.setAttribute('data-theme', t); } else { root.removeAttribute('data-theme'); } } - function current() { return root.getAttribute('data-theme') || 'auto'; } + function apply(t) { if (t === 'light' || t === 'dark') { root.dataset.theme = t; } else { delete root.dataset.theme; } } + function current() { return root.dataset.theme || 'auto'; } + function store(theme) { + // localStorage throws when it is unavailable (e.g. private mode). Report that to the caller instead of + // swallowing it in an empty catch — persisting the theme is best effort, so the caller ignores the result. + try { + if (theme === 'auto') { localStorage.removeItem(KEY); } else { localStorage.setItem(KEY, theme); } + return true; + } catch { + return false; + } + } var btn = document.getElementById('theme-toggle'); if (btn) { btn.addEventListener('click', function () { var order = ['auto', 'light', 'dark']; var next = order[(order.indexOf(current()) + 1) % order.length]; - try { if (next === 'auto') { localStorage.removeItem(KEY); } else { localStorage.setItem(KEY, next); } } catch (e) {} + store(next); apply(next); }); } @@ -159,21 +169,21 @@

BANK_TRANSACTION_FILE_DATE_OUT_OF_STATEMENT_PERIOD var q = ((search && search.value) || '').toLowerCase().trim(); var items = toc.querySelectorAll('.toc-item'); var visible = 0; - for (var i = 0; i < items.length; i++) { - var ok = !q || (items[i].getAttribute('data-search') || '').indexOf(q) !== -1; - items[i].hidden = !ok; + for (var item of items) { + var ok = !q || (item.dataset.search || '').includes(q); + item.hidden = !ok; if (ok) { visible++; } } var groups = toc.querySelectorAll('.toc-group'); - for (var g = 0; g < groups.length; g++) { - groups[g].hidden = groups[g].querySelectorAll('.toc-item:not([hidden])').length === 0; + for (var group of groups) { + group.hidden = group.querySelectorAll('.toc-item:not([hidden])').length === 0; } if (noResults) { noResults.hidden = visible !== 0; } } if (search) { search.addEventListener('input', filter); } var anchors = document.querySelectorAll('a[data-anchor]'); - for (var j = 0; j < anchors.length; j++) { - anchors[j].addEventListener('click', function () { + for (var anchor of anchors) { + anchor.addEventListener('click', function () { var href = this.getAttribute('href') || ''; var url = location.href.split('#')[0] + href; if (navigator.clipboard && navigator.clipboard.writeText) { navigator.clipboard.writeText(url).catch(function () {}); } diff --git a/FirstClassErrors.GenDoc.UnitTests/UsageDocumentationSnapshotTests.TheSplitHtmlRenderingOfTheUsageCatalog#errors-BANK_TRANSACTION_FILE_STATEMENT_TOTAL_AMOUNT_MISMATCH.verified.html b/FirstClassErrors.GenDoc.UnitTests/UsageDocumentationSnapshotTests.TheSplitHtmlRenderingOfTheUsageCatalog#errors-BANK_TRANSACTION_FILE_STATEMENT_TOTAL_AMOUNT_MISMATCH.verified.html index 035a54a..eee7425 100644 --- a/FirstClassErrors.GenDoc.UnitTests/UsageDocumentationSnapshotTests.TheSplitHtmlRenderingOfTheUsageCatalog#errors-BANK_TRANSACTION_FILE_STATEMENT_TOTAL_AMOUNT_MISMATCH.verified.html +++ b/FirstClassErrors.GenDoc.UnitTests/UsageDocumentationSnapshotTests.TheSplitHtmlRenderingOfTheUsageCatalog#errors-BANK_TRANSACTION_FILE_STATEMENT_TOTAL_AMOUNT_MISMATCH.verified.html @@ -81,7 +81,7 @@ .group-title { font-size: 1.3rem; } } - + @@ -130,14 +130,24 @@

BANK_TRANSACTION_FILE_STATEMENT_TOTAL_AMOUNT_MISMA (function () { var KEY = 'fce-theme'; var root = document.documentElement; - function apply(t) { if (t === 'light' || t === 'dark') { root.setAttribute('data-theme', t); } else { root.removeAttribute('data-theme'); } } - function current() { return root.getAttribute('data-theme') || 'auto'; } + function apply(t) { if (t === 'light' || t === 'dark') { root.dataset.theme = t; } else { delete root.dataset.theme; } } + function current() { return root.dataset.theme || 'auto'; } + function store(theme) { + // localStorage throws when it is unavailable (e.g. private mode). Report that to the caller instead of + // swallowing it in an empty catch — persisting the theme is best effort, so the caller ignores the result. + try { + if (theme === 'auto') { localStorage.removeItem(KEY); } else { localStorage.setItem(KEY, theme); } + return true; + } catch { + return false; + } + } var btn = document.getElementById('theme-toggle'); if (btn) { btn.addEventListener('click', function () { var order = ['auto', 'light', 'dark']; var next = order[(order.indexOf(current()) + 1) % order.length]; - try { if (next === 'auto') { localStorage.removeItem(KEY); } else { localStorage.setItem(KEY, next); } } catch (e) {} + store(next); apply(next); }); } @@ -149,21 +159,21 @@

BANK_TRANSACTION_FILE_STATEMENT_TOTAL_AMOUNT_MISMA var q = ((search && search.value) || '').toLowerCase().trim(); var items = toc.querySelectorAll('.toc-item'); var visible = 0; - for (var i = 0; i < items.length; i++) { - var ok = !q || (items[i].getAttribute('data-search') || '').indexOf(q) !== -1; - items[i].hidden = !ok; + for (var item of items) { + var ok = !q || (item.dataset.search || '').includes(q); + item.hidden = !ok; if (ok) { visible++; } } var groups = toc.querySelectorAll('.toc-group'); - for (var g = 0; g < groups.length; g++) { - groups[g].hidden = groups[g].querySelectorAll('.toc-item:not([hidden])').length === 0; + for (var group of groups) { + group.hidden = group.querySelectorAll('.toc-item:not([hidden])').length === 0; } if (noResults) { noResults.hidden = visible !== 0; } } if (search) { search.addEventListener('input', filter); } var anchors = document.querySelectorAll('a[data-anchor]'); - for (var j = 0; j < anchors.length; j++) { - anchors[j].addEventListener('click', function () { + for (var anchor of anchors) { + anchor.addEventListener('click', function () { var href = this.getAttribute('href') || ''; var url = location.href.split('#')[0] + href; if (navigator.clipboard && navigator.clipboard.writeText) { navigator.clipboard.writeText(url).catch(function () {}); } diff --git a/FirstClassErrors.GenDoc.UnitTests/UsageDocumentationSnapshotTests.TheSplitHtmlRenderingOfTheUsageCatalog#errors-EXCHANGE_RATE_SERVICE_UNAVAILABLE.verified.html b/FirstClassErrors.GenDoc.UnitTests/UsageDocumentationSnapshotTests.TheSplitHtmlRenderingOfTheUsageCatalog#errors-EXCHANGE_RATE_SERVICE_UNAVAILABLE.verified.html index a5af09f..6660d0e 100644 --- a/FirstClassErrors.GenDoc.UnitTests/UsageDocumentationSnapshotTests.TheSplitHtmlRenderingOfTheUsageCatalog#errors-EXCHANGE_RATE_SERVICE_UNAVAILABLE.verified.html +++ b/FirstClassErrors.GenDoc.UnitTests/UsageDocumentationSnapshotTests.TheSplitHtmlRenderingOfTheUsageCatalog#errors-EXCHANGE_RATE_SERVICE_UNAVAILABLE.verified.html @@ -81,7 +81,7 @@ .group-title { font-size: 1.3rem; } } - + @@ -139,14 +139,24 @@

EXCHANGE_RATE_SERVICE_UNAVAILABLE EXCHANGE_RATE_SERVICE_UNAVAILABLE - + @@ -138,14 +138,24 @@

MALFORMED_STATEMENT_PAYLOAD Skip to content @@ -137,14 +137,24 @@

MONEY_TRANSFER_AMOUNT_NOT_POSITIVE MONEY_TRANSFER_AMOUNT_NOT_POSITIVE - + @@ -127,14 +127,24 @@

MONEY_TRANSFER_INVALID MONEY_TRANSFER_INVALID - + @@ -137,14 +137,24 @@

STATEMENT_UPLOAD_RATE_LIMITED Skip to content @@ -142,14 +142,24 @@

TEMPERATURE_BELOW_ABSOLUTE_ZERO Skip to content @@ -138,14 +138,24 @@

UNSUPPORTED_CURRENCY_PAIR UNSUPPORTED_CURRENCY_PAIR - + @@ -146,14 +146,24 @@

Error Catalog

(function () { var KEY = 'fce-theme'; var root = document.documentElement; - function apply(t) { if (t === 'light' || t === 'dark') { root.setAttribute('data-theme', t); } else { root.removeAttribute('data-theme'); } } - function current() { return root.getAttribute('data-theme') || 'auto'; } + function apply(t) { if (t === 'light' || t === 'dark') { root.dataset.theme = t; } else { delete root.dataset.theme; } } + function current() { return root.dataset.theme || 'auto'; } + function store(theme) { + // localStorage throws when it is unavailable (e.g. private mode). Report that to the caller instead of + // swallowing it in an empty catch — persisting the theme is best effort, so the caller ignores the result. + try { + if (theme === 'auto') { localStorage.removeItem(KEY); } else { localStorage.setItem(KEY, theme); } + return true; + } catch { + return false; + } + } var btn = document.getElementById('theme-toggle'); if (btn) { btn.addEventListener('click', function () { var order = ['auto', 'light', 'dark']; var next = order[(order.indexOf(current()) + 1) % order.length]; - try { if (next === 'auto') { localStorage.removeItem(KEY); } else { localStorage.setItem(KEY, next); } } catch (e) {} + store(next); apply(next); }); } @@ -165,21 +175,21 @@

Error Catalog

var q = ((search && search.value) || '').toLowerCase().trim(); var items = toc.querySelectorAll('.toc-item'); var visible = 0; - for (var i = 0; i < items.length; i++) { - var ok = !q || (items[i].getAttribute('data-search') || '').indexOf(q) !== -1; - items[i].hidden = !ok; + for (var item of items) { + var ok = !q || (item.dataset.search || '').includes(q); + item.hidden = !ok; if (ok) { visible++; } } var groups = toc.querySelectorAll('.toc-group'); - for (var g = 0; g < groups.length; g++) { - groups[g].hidden = groups[g].querySelectorAll('.toc-item:not([hidden])').length === 0; + for (var group of groups) { + group.hidden = group.querySelectorAll('.toc-item:not([hidden])').length === 0; } if (noResults) { noResults.hidden = visible !== 0; } } if (search) { search.addEventListener('input', filter); } var anchors = document.querySelectorAll('a[data-anchor]'); - for (var j = 0; j < anchors.length; j++) { - anchors[j].addEventListener('click', function () { + for (var anchor of anchors) { + anchor.addEventListener('click', function () { var href = this.getAttribute('href') || ''; var url = location.href.split('#')[0] + href; if (navigator.clipboard && navigator.clipboard.writeText) { navigator.clipboard.writeText(url).catch(function () {}); } diff --git a/FirstClassErrors.GenDoc.Worker/Program.cs b/FirstClassErrors.GenDoc.Worker/Program.cs index b0734f0..2af528f 100644 --- a/FirstClassErrors.GenDoc.Worker/Program.cs +++ b/FirstClassErrors.GenDoc.Worker/Program.cs @@ -1,5 +1,6 @@ #region Usings declarations +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Reflection; using System.Text.Json; @@ -23,34 +24,16 @@ // // Exit codes: 0 = success, 1 = fatal extraction error, 2 = bad usage. -string? assemblyPath = null; -string? outputPath = null; -string? cultureName = null; +(string? assemblyPath, string? outputPath, string? cultureName, string? parseError) = ParseArguments(args); -for (int index = 0; index < args.Length; index++) { - string arg = args[index]; +if (parseError is not null) { + await Console.Error.WriteLineAsync(parseError); - if (string.Equals(arg, "--culture", StringComparison.Ordinal)) { - if (index + 1 >= args.Length) { - Console.Error.WriteLine("Missing value for --culture."); - - return 2; - } - - cultureName = args[++index]; - - continue; - } - - if (assemblyPath is null) { - assemblyPath = arg; - } else if (outputPath is null) { - outputPath = arg; - } + return 2; } if (string.IsNullOrWhiteSpace(assemblyPath)) { - Console.Error.WriteLine("Usage: FirstClassErrors.GenDoc.Worker [output-json-path] [--culture ]"); + await Console.Error.WriteLineAsync("Usage: FirstClassErrors.GenDoc.Worker [output-json-path] [--culture ]"); return 2; } @@ -58,19 +41,19 @@ if (cultureName is not null) { try { CultureInfo culture = CultureInfo.GetCultureInfo(cultureName); - CultureInfo.CurrentCulture = culture; - CultureInfo.CurrentUICulture = culture; + CultureInfo.CurrentCulture = culture; + CultureInfo.CurrentUICulture = culture; CultureInfo.DefaultThreadCurrentCulture = culture; CultureInfo.DefaultThreadCurrentUICulture = culture; } catch (CultureNotFoundException) { - Console.Error.WriteLine($"Unknown culture '{cultureName}'."); + await Console.Error.WriteLineAsync($"Unknown culture '{cultureName}'."); return 2; } } try { - Assembly assembly = Assembly.LoadFrom(assemblyPath); + Assembly assembly = LoadTarget(assemblyPath); ErrorDocumentationExtractionResult result = AssemblyErrorDocumentationReader.GetErrorDocumentationFrom(assembly); JsonSerializerOptions options = new() { @@ -81,14 +64,62 @@ string json = JsonSerializer.Serialize(result, options); if (outputPath is null) { - Console.Out.Write(json); + await Console.Out.WriteAsync(json); } else { - File.WriteAllText(outputPath, json); + await File.WriteAllTextAsync(outputPath, json); } return 0; } catch (Exception ex) { - Console.Error.WriteLine($"Fatal error while extracting documentation from '{assemblyPath}': {ex}"); + await Console.Error.WriteLineAsync($"Fatal error while extracting documentation from '{assemblyPath}': {ex}"); return 1; } + +// Parses the positional [output-json-path] and the optional --culture . Returns the parsed +// values plus a non-null error message when --culture is given without a value. Kept as a local function so the +// top-level flow stays a straight-line sequence of guards. +static (string? assemblyPath, string? outputPath, string? cultureName, string? error) ParseArguments(string[] arguments) { + string? assemblyPath = null; + string? outputPath = null; + string? cultureName = null; + + int index = 0; + while (index < arguments.Length) { + string arg = arguments[index]; + + if (string.Equals(arg, "--culture", StringComparison.Ordinal)) { + if (index + 1 >= arguments.Length) { + return (null, null, null, "Missing value for --culture."); + } + + cultureName = arguments[index + 1]; + index += 2; + + continue; + } + + if (assemblyPath is null) { + assemblyPath = arg; + } else if (outputPath is null) { + outputPath = arg; + } + + index++; + } + + return (assemblyPath, outputPath, cultureName, null); +} + +// Assembly.LoadFrom is deliberate here (S3885): the worker documents a target given only by a path, and the +// generator runs it WITHOUT a deps.json when the target has none. LoadFrom probes the target's own directory for +// the target's co-located dependencies; loading into the default context (LoadFromAssemblyPath) would not, so a +// documentation factory referencing a sibling DLL would fail to resolve it. +[SuppressMessage("Major Code Smell", "S3885:\"Assembly.Load\" should be used instead of \"Assembly.LoadFrom\"", + Justification = + "LoadFrom probes the target's own directory for its co-located dependencies, which the deps.json-optional " + + "worker relies on when documenting a prebuilt assembly that ships no deps.json. Assembly.Load resolves by " + + "name, not path, and cannot load the target at all.")] +static Assembly LoadTarget(string assemblyPath) { + return Assembly.LoadFrom(assemblyPath); +} diff --git a/FirstClassErrors.GenDoc/Rendering/HtmlErrorDocumentationRenderer.cs b/FirstClassErrors.GenDoc/Rendering/HtmlErrorDocumentationRenderer.cs index 05af57a..111e9a7 100644 --- a/FirstClassErrors.GenDoc/Rendering/HtmlErrorDocumentationRenderer.cs +++ b/FirstClassErrors.GenDoc/Rendering/HtmlErrorDocumentationRenderer.cs @@ -34,10 +34,10 @@ public sealed class HtmlErrorDocumentationRenderer : IErrorDocumentationRenderer /// public IReadOnlyList Render(IEnumerable catalog, RenderRequest request) { - if (catalog is null) { throw new ArgumentNullException(nameof(catalog)); } - if (request is null) { throw new ArgumentNullException(nameof(request)); } + ArgumentNullException.ThrowIfNull(catalog); + ArgumentNullException.ThrowIfNull(request); - if (SupportedLayouts.Contains(request.Layout, StringComparer.OrdinalIgnoreCase) is false) { + if (!SupportedLayouts.Contains(request.Layout, StringComparer.OrdinalIgnoreCase)) { throw new LayoutNotSupportedException(Format, request.Layout, SupportedLayouts); } @@ -216,11 +216,11 @@ private static void AppendErrorDetail(StringBuilder html, Entry entry, HtmlRende html.Append($"<{h} class=\"error-title\">{Text(entry.Code)} #\n"); html.Append($"

{Text(entry.Title)}

\n"); - if (string.IsNullOrWhiteSpace(error.Explanation) is false) { + if (!string.IsNullOrWhiteSpace(error.Explanation)) { html.Append($"
<{hSub}>{Text(strings.DocumentationHeading)}

{Text(error.Explanation!.Trim())}

\n"); } - if (string.IsNullOrWhiteSpace(error.BusinessRule) is false) { + if (!string.IsNullOrWhiteSpace(error.BusinessRule)) { html.Append($"
{Text(strings.BusinessRuleLabel)} {Text(error.BusinessRule!.Trim())}
\n"); } @@ -298,11 +298,11 @@ private static string ProblemDetailsJson(ErrorDescription example, string? code, } json.Append($" \"title\": \"{JsonString(example.ShortMessage)}\""); - if (string.IsNullOrWhiteSpace(example.DetailedMessage) is false) { + if (!string.IsNullOrWhiteSpace(example.DetailedMessage)) { json.Append($",\n \"detail\": \"{JsonString(example.DetailedMessage!)}\""); } - if (string.IsNullOrWhiteSpace(code) is false) { + if (!string.IsNullOrWhiteSpace(code)) { json.Append($",\n \"code\": \"{JsonString(code!.Trim())}\""); } @@ -318,10 +318,10 @@ private static string ProblemDetailsJson(ErrorDescription example, string? code, private static string DiagnosticLogLine(ErrorDescription example, string? code, string? source) { StringBuilder line = new(); line.Append("2026-07-04T13:42:18.734Z ERROR"); - if (string.IsNullOrWhiteSpace(source) is false) { line.Append($" [{source!.Trim()}]"); } + if (!string.IsNullOrWhiteSpace(source)) { line.Append($" [{source!.Trim()}]"); } line.Append($" {Inline(example.DiagnosticMessage)}"); - if (string.IsNullOrWhiteSpace(code) is false) { line.Append($" error.code={code!.Trim()}"); } + if (!string.IsNullOrWhiteSpace(code)) { line.Append($" error.code={code!.Trim()}"); } return line.ToString(); } @@ -340,7 +340,7 @@ private static string JsonString(string value) { #region Helpers - private static IReadOnlyList BuildEntries(IEnumerable catalog) { + private static List BuildEntries(IEnumerable catalog) { List entries = []; HashSet usedNames = new(StringComparer.OrdinalIgnoreCase); @@ -365,7 +365,7 @@ private static IReadOnlyList BuildEntries(IEnumerable string name = baseName; int suffix = 2; - while (usedNames.Add(name) is false) { + while (!usedNames.Add(name)) { name = $"{baseName}-{suffix}"; suffix++; } @@ -376,7 +376,7 @@ private static IReadOnlyList BuildEntries(IEnumerable return entries; } - private static IReadOnlyList GroupBySource(IReadOnlyList entries) { + private static List GroupBySource(IReadOnlyList entries) { List groups = []; Dictionary byKey = new(StringComparer.Ordinal); HashSet usedAnchors = new(StringComparer.OrdinalIgnoreCase); @@ -384,7 +384,7 @@ private static IReadOnlyList GroupBySource(IReadOnlyList entries) // Group by source (ProvidesErrorsFor target), preserving first-seen order of both groups and errors. foreach (Entry entry in entries) { string source = entry.Source ?? "Other"; - if (byKey.TryGetValue(source, out Group? group) is false) { + if (!byKey.TryGetValue(source, out Group? group)) { group = new Group(source, UniqueAnchor(source, usedAnchors), []); byKey[source] = group; groups.Add(group); @@ -403,7 +403,7 @@ private static string UniqueAnchor(string source, HashSet used) { string anchor = "src-" + stem; int suffix = 2; - while (used.Add(anchor) is false) { + while (!used.Add(anchor)) { anchor = $"src-{stem}-{suffix}"; suffix++; } @@ -441,22 +441,16 @@ private static string HtmlLang(CultureInfo culture) { } private static string? FirstNonEmpty(params string?[] values) { - foreach (string? value in values) { - if (string.IsNullOrWhiteSpace(value) is false) { return value.Trim(); } - } + string? value = values.FirstOrDefault(candidate => !string.IsNullOrWhiteSpace(candidate)); - return null; + return value?.Trim(); } /// Gets the group's source description (shared by its errors), or null when none is set. private static string? GroupDescription(Group group) { - foreach (Entry entry in group.Entries) { - if (string.IsNullOrWhiteSpace(entry.Error.SourceDescription) is false) { - return entry.Error.SourceDescription!.Trim(); - } - } + Entry? described = group.Entries.FirstOrDefault(entry => !string.IsNullOrWhiteSpace(entry.Error.SourceDescription)); - return null; + return described?.Error.SourceDescription!.Trim(); } /// Turns a code or source name into a safe file-name / anchor stem (letters, digits, ._-). @@ -467,7 +461,7 @@ private static string SafeStem(string value) { if (char.IsAsciiLetterOrDigit(character) || character is '_' or '-' or '.') { builder.Append(character); lastDash = false; - } else if (lastDash is false && builder.Length > 0) { + } else if (!lastDash && builder.Length > 0) { builder.Append('-'); lastDash = true; } @@ -499,7 +493,7 @@ private static string CodeCell(string? value) { } private static string ExampleValuesCell(IReadOnlyList values) { - IEnumerable rendered = values.Where(value => string.IsNullOrWhiteSpace(value) is false) + IEnumerable rendered = values.Where(value => !string.IsNullOrWhiteSpace(value)) .Select(value => $"{Text(value!.Trim())}"); return string.Join(", ", rendered); @@ -560,7 +554,7 @@ private static string BuildSearchText(ErrorDocumentation error, string code, str parts.Add(contextEntry.Description); } - string joined = string.Join(" ", parts.Where(p => string.IsNullOrWhiteSpace(p) is false) + string joined = string.Join(" ", parts.Where(p => !string.IsNullOrWhiteSpace(p)) .Select(p => p!.Trim().Replace("\r", " ").Replace("\n", " "))); return joined.ToLowerInvariant(); diff --git a/FirstClassErrors.GenDoc/Rendering/HtmlRendererAssets.cs b/FirstClassErrors.GenDoc/Rendering/HtmlRendererAssets.cs index 707c21d..dbbaeb5 100644 --- a/FirstClassErrors.GenDoc/Rendering/HtmlRendererAssets.cs +++ b/FirstClassErrors.GenDoc/Rendering/HtmlRendererAssets.cs @@ -9,7 +9,7 @@ internal static class HtmlRendererAssets { /// Inline <head> script that applies the stored theme before first paint (avoids a flash). public const string ThemeInit = - """(function(){try{var t=localStorage.getItem('fce-theme');if(t==='light'||t==='dark'){document.documentElement.setAttribute('data-theme',t);}}catch(e){}})();"""; + """(function(){try{var t=localStorage.getItem('fce-theme');if(t==='light'||t==='dark'){document.documentElement.dataset.theme=t;}}catch(e){/* localStorage may be unavailable (e.g. private mode); fall back to the default theme */}})();"""; /// The theme-toggle button icon (inline SVG, inherits the current color). public const string ThemeIcon = @@ -101,14 +101,24 @@ internal static class HtmlRendererAssets { (function () { var KEY = 'fce-theme'; var root = document.documentElement; - function apply(t) { if (t === 'light' || t === 'dark') { root.setAttribute('data-theme', t); } else { root.removeAttribute('data-theme'); } } - function current() { return root.getAttribute('data-theme') || 'auto'; } + function apply(t) { if (t === 'light' || t === 'dark') { root.dataset.theme = t; } else { delete root.dataset.theme; } } + function current() { return root.dataset.theme || 'auto'; } + function store(theme) { + // localStorage throws when it is unavailable (e.g. private mode). Report that to the caller instead of + // swallowing it in an empty catch — persisting the theme is best effort, so the caller ignores the result. + try { + if (theme === 'auto') { localStorage.removeItem(KEY); } else { localStorage.setItem(KEY, theme); } + return true; + } catch { + return false; + } + } var btn = document.getElementById('theme-toggle'); if (btn) { btn.addEventListener('click', function () { var order = ['auto', 'light', 'dark']; var next = order[(order.indexOf(current()) + 1) % order.length]; - try { if (next === 'auto') { localStorage.removeItem(KEY); } else { localStorage.setItem(KEY, next); } } catch (e) {} + store(next); apply(next); }); } @@ -120,21 +130,21 @@ function filter() { var q = ((search && search.value) || '').toLowerCase().trim(); var items = toc.querySelectorAll('.toc-item'); var visible = 0; - for (var i = 0; i < items.length; i++) { - var ok = !q || (items[i].getAttribute('data-search') || '').indexOf(q) !== -1; - items[i].hidden = !ok; + for (var item of items) { + var ok = !q || (item.dataset.search || '').includes(q); + item.hidden = !ok; if (ok) { visible++; } } var groups = toc.querySelectorAll('.toc-group'); - for (var g = 0; g < groups.length; g++) { - groups[g].hidden = groups[g].querySelectorAll('.toc-item:not([hidden])').length === 0; + for (var group of groups) { + group.hidden = group.querySelectorAll('.toc-item:not([hidden])').length === 0; } if (noResults) { noResults.hidden = visible !== 0; } } if (search) { search.addEventListener('input', filter); } var anchors = document.querySelectorAll('a[data-anchor]'); - for (var j = 0; j < anchors.length; j++) { - anchors[j].addEventListener('click', function () { + for (var anchor of anchors) { + anchor.addEventListener('click', function () { var href = this.getAttribute('href') || ''; var url = location.href.split('#')[0] + href; if (navigator.clipboard && navigator.clipboard.writeText) { navigator.clipboard.writeText(url).catch(function () {}); } diff --git a/FirstClassErrors.GenDoc/Rendering/JsonErrorDocumentationRenderer.cs b/FirstClassErrors.GenDoc/Rendering/JsonErrorDocumentationRenderer.cs index 860f1cc..5d80e60 100644 --- a/FirstClassErrors.GenDoc/Rendering/JsonErrorDocumentationRenderer.cs +++ b/FirstClassErrors.GenDoc/Rendering/JsonErrorDocumentationRenderer.cs @@ -34,11 +34,11 @@ public sealed class JsonErrorDocumentationRenderer : IErrorDocumentationRenderer /// public IReadOnlyList Render(IEnumerable catalog, RenderRequest request) { - if (catalog is null) { throw new ArgumentNullException(nameof(catalog)); } - if (request is null) { throw new ArgumentNullException(nameof(request)); } + ArgumentNullException.ThrowIfNull(catalog); + ArgumentNullException.ThrowIfNull(request); // A single JSON catalog has no notion of a split layout; reject anything but the layouts we advertise. - if (SupportedLayouts.Contains(request.Layout, StringComparer.OrdinalIgnoreCase) is false) { + if (!SupportedLayouts.Contains(request.Layout, StringComparer.OrdinalIgnoreCase)) { throw new LayoutNotSupportedException(Format, request.Layout, SupportedLayouts); } diff --git a/FirstClassErrors.GenDoc/Rendering/MarkdownErrorDocumentationRenderer.cs b/FirstClassErrors.GenDoc/Rendering/MarkdownErrorDocumentationRenderer.cs index c60f652..bba59b9 100644 --- a/FirstClassErrors.GenDoc/Rendering/MarkdownErrorDocumentationRenderer.cs +++ b/FirstClassErrors.GenDoc/Rendering/MarkdownErrorDocumentationRenderer.cs @@ -23,10 +23,10 @@ public sealed class MarkdownErrorDocumentationRenderer : IErrorDocumentationRend /// public IReadOnlyList Render(IEnumerable catalog, RenderRequest request) { - if (catalog is null) { throw new ArgumentNullException(nameof(catalog)); } - if (request is null) { throw new ArgumentNullException(nameof(request)); } + ArgumentNullException.ThrowIfNull(catalog); + ArgumentNullException.ThrowIfNull(request); - if (SupportedLayouts.Contains(request.Layout, StringComparer.OrdinalIgnoreCase) is false) { + if (!SupportedLayouts.Contains(request.Layout, StringComparer.OrdinalIgnoreCase)) { throw new LayoutNotSupportedException(Format, request.Layout, SupportedLayouts); } @@ -86,7 +86,7 @@ private static IReadOnlyList RenderSingle(IReadOnlyList return [new RenderedDocument("errors.md", markdown.ToString())]; } - private static IReadOnlyList RenderSplit(IReadOnlyList entries, MarkdownRendererStrings strings, string? serviceName) { + private static List RenderSplit(IReadOnlyList entries, MarkdownRendererStrings strings, string? serviceName) { List documents = []; StringBuilder index = new(); @@ -143,18 +143,18 @@ private static void AppendErrorBody(StringBuilder markdown, Entry entry, int hea markdown.Append($"{heading} {Inline(entry.Title)}\n\n"); - bool hasCode = string.IsNullOrWhiteSpace(error.Code) is false; - bool hasSource = string.IsNullOrWhiteSpace(error.Source) is false; + bool hasCode = !string.IsNullOrWhiteSpace(error.Code); + bool hasSource = !string.IsNullOrWhiteSpace(error.Source); if (hasCode) { markdown.Append($"- **{strings.CodeLabel}** {CodeSpan(error.Code!.Trim())}\n"); } if (hasSource) { markdown.Append($"- **{strings.SourceLabel}** {CodeSpan(error.Source!.Trim())}\n"); } if (hasCode || hasSource) { markdown.Append('\n'); } - if (string.IsNullOrWhiteSpace(error.Explanation) is false) { + if (!string.IsNullOrWhiteSpace(error.Explanation)) { // A paragraph: keep author line breaks intact. markdown.Append(error.Explanation!.Trim()).Append("\n\n"); } - if (string.IsNullOrWhiteSpace(error.BusinessRule) is false) { + if (!string.IsNullOrWhiteSpace(error.BusinessRule)) { markdown.Append($"> **{strings.BusinessRuleLabel}** {Inline(error.BusinessRule)}\n\n"); } @@ -215,11 +215,11 @@ private static string ProblemDetailsJson(ErrorDescription example, string? code, } json.Append($" \"title\": \"{JsonString(example.ShortMessage)}\""); - if (string.IsNullOrWhiteSpace(example.DetailedMessage) is false) { + if (!string.IsNullOrWhiteSpace(example.DetailedMessage)) { json.Append($",\n \"detail\": \"{JsonString(example.DetailedMessage!)}\""); } - if (string.IsNullOrWhiteSpace(code) is false) { + if (!string.IsNullOrWhiteSpace(code)) { json.Append($",\n \"code\": \"{JsonString(code!.Trim())}\""); } @@ -242,12 +242,12 @@ private static string ProblemDetailsJson(ErrorDescription example, string? code, private static string DiagnosticLogLine(ErrorDescription example, string? code, string? source) { StringBuilder line = new(); line.Append($"{SampleLogTimestamp} ERROR"); - if (string.IsNullOrWhiteSpace(source) is false) { + if (!string.IsNullOrWhiteSpace(source)) { line.Append($" [{source!.Trim()}]"); } line.Append($" {Inline(example.DiagnosticMessage)}"); - if (string.IsNullOrWhiteSpace(code) is false) { + if (!string.IsNullOrWhiteSpace(code)) { line.Append($" error.code={code!.Trim()}"); } @@ -263,7 +263,7 @@ private static string JsonString(string value) { return JsonEncodedText.Encode(Inline(value), JavaScriptEncoder.UnsafeRelaxedJsonEscaping).ToString(); } - private static IReadOnlyList BuildEntries(IEnumerable catalog) { + private static List BuildEntries(IEnumerable catalog) { List entries = []; HashSet usedSlugs = new(StringComparer.OrdinalIgnoreCase); @@ -277,7 +277,7 @@ private static IReadOnlyList BuildEntries(IEnumerable string slug = baseSlug; int suffix = 2; - while (usedSlugs.Add(slug) is false) { + while (!usedSlugs.Add(slug)) { slug = $"{baseSlug}-{suffix}"; suffix++; } @@ -288,7 +288,7 @@ private static IReadOnlyList BuildEntries(IEnumerable return entries; } - private static IReadOnlyList GroupBySource(IReadOnlyList entries, MarkdownRendererStrings strings) { + private static List GroupBySource(IReadOnlyList entries, MarkdownRendererStrings strings) { List groups = []; Dictionary byKey = new(StringComparer.Ordinal); @@ -296,7 +296,7 @@ private static IReadOnlyList GroupBySource(IReadOnlyList entries, // The label is localized, but the anchor and file name stay culture-invariant so links are stable across languages. foreach (Entry entry in entries) { string source = FirstNonEmpty(entry.Error.Source) ?? "Other"; - if (byKey.TryGetValue(source, out Group? group) is false) { + if (!byKey.TryGetValue(source, out Group? group)) { string slug = SlugifySource(source); group = new Group(strings.GroupLabel(source), $"src-{slug}", $"{slug}-errors.md", []); byKey[source] = group; @@ -310,11 +310,9 @@ private static IReadOnlyList GroupBySource(IReadOnlyList entries, } private static string? FirstNonEmpty(params string?[] values) { - foreach (string? value in values) { - if (string.IsNullOrWhiteSpace(value) is false) { return value.Trim(); } - } + string? value = values.FirstOrDefault(candidate => !string.IsNullOrWhiteSpace(candidate)); - return null; + return value?.Trim(); } private static string Slugify(string value) { @@ -324,7 +322,7 @@ private static string Slugify(string value) { if (character is (>= 'a' and <= 'z') or (>= '0' and <= '9')) { builder.Append(character); lastDash = false; - } else if (lastDash is false && builder.Length > 0) { + } else if (!lastDash && builder.Length > 0) { builder.Append('-'); lastDash = true; } @@ -355,13 +353,9 @@ private static string SlugifySource(string source) { /// Gets the group's source description (shared by its errors), or null when none is set. private static string? GroupDescription(Group group) { - foreach (Entry entry in group.Entries) { - if (string.IsNullOrWhiteSpace(entry.Error.SourceDescription) is false) { - return entry.Error.SourceDescription!.Trim(); - } - } + Entry? described = group.Entries.FirstOrDefault(entry => !string.IsNullOrWhiteSpace(entry.Error.SourceDescription)); - return null; + return described?.Error.SourceDescription!.Trim(); } /// Trims a value and folds any line breaks into spaces so it stays on one Markdown line. @@ -417,7 +411,7 @@ private static int LongestRun(string value, char c) { } private static string ExampleValuesCell(IReadOnlyList values) { - IEnumerable rendered = values.Where(value => string.IsNullOrWhiteSpace(value) is false) + IEnumerable rendered = values.Where(value => !string.IsNullOrWhiteSpace(value)) .Select(value => CodeCell(value)); return string.Join(", ", rendered); diff --git a/FirstClassErrors.GenDoc/Rendering/ProblemType.cs b/FirstClassErrors.GenDoc/Rendering/ProblemType.cs index f275e5b..2867bdc 100644 --- a/FirstClassErrors.GenDoc/Rendering/ProblemType.cs +++ b/FirstClassErrors.GenDoc/Rendering/ProblemType.cs @@ -55,7 +55,7 @@ private static string Slugify(string? value) { if (character is (>= 'a' and <= 'z') or (>= '0' and <= '9')) { builder.Append(character); lastDash = false; - } else if (lastDash is false && builder.Length > 0) { + } else if (!lastDash && builder.Length > 0) { builder.Append('-'); lastDash = true; } diff --git a/FirstClassErrors.GenDoc/SolutionErrorDocumentationGenerator.cs b/FirstClassErrors.GenDoc/SolutionErrorDocumentationGenerator.cs index 3942d44..52799fd 100644 --- a/FirstClassErrors.GenDoc/SolutionErrorDocumentationGenerator.cs +++ b/FirstClassErrors.GenDoc/SolutionErrorDocumentationGenerator.cs @@ -17,6 +17,9 @@ public static class SolutionErrorDocumentationGenerator { #region Statics members declarations + /// The .NET CLI executable used for every solution/project SDK query and for the build. + private const string DotNetCli = "dotnet"; + public static IEnumerable GetErrorDocumentationFrom(string solutionPath) { ArgumentNullException.ThrowIfNull(solutionPath); @@ -30,7 +33,7 @@ public static IEnumerable GetErrorDocumentationFrom(string s options.Logger.Info($"Starting documentation generation for solution '{solutionPath}'"); string fullSolutionPath = Path.GetFullPath(solutionPath); - if (File.Exists(fullSolutionPath) is false) { throw new FileNotFoundException($"Solution file not found: '{fullSolutionPath}'", fullSolutionPath); } + if (!File.Exists(fullSolutionPath)) { throw new FileNotFoundException($"Solution file not found: '{fullSolutionPath}'", fullSolutionPath); } // Accept both the classic (.sln) and the XML (.slnx) solution formats: "dotnet sln list", used below to // enumerate the projects, handles the two uniformly. Solution filters (.slnf) are intentionally excluded — @@ -38,13 +41,13 @@ public static IEnumerable GetErrorDocumentationFrom(string s string extension = Path.GetExtension(fullSolutionPath); bool isSolution = string.Equals(extension, ".sln", StringComparison.OrdinalIgnoreCase) || string.Equals(extension, ".slnx", StringComparison.OrdinalIgnoreCase); - if (isSolution is false) { throw new ArgumentException($"Expected a .sln or .slnx file path, got: '{fullSolutionPath}'", nameof(solutionPath)); } + if (!isSolution) { throw new ArgumentException($"Expected a .sln or .slnx file path, got: '{fullSolutionPath}'", nameof(solutionPath)); } if (options.BuildSolution) { DotNetBuild(fullSolutionPath, options); } - IReadOnlyList projects = ReadSolutionProjects(fullSolutionPath, options); + List projects = ReadSolutionProjects(fullSolutionPath, options); options.Logger.Debug($"Found {projects.Count} MSBuild projects in solution."); IReadOnlyList includedProjects = FilterProjects(projects, options); @@ -91,7 +94,7 @@ public static IEnumerable GetErrorDocumentationFromAssemblie List resolved = new(); foreach (string assemblyPath in assemblyPaths) { string fullPath = Path.GetFullPath(assemblyPath); - if (File.Exists(fullPath) is false) { + if (!File.Exists(fullPath)) { HandleFailure(options, $"Assembly not found: '{fullPath}'."); continue; @@ -114,7 +117,7 @@ private static List ReadSolutionProjects(string solutionPath, SolutionGe // the generator free of the heavy Microsoft.Build dependency and handles both .sln and .slnx uniformly. string solutionDirectory = Path.GetDirectoryName(solutionPath) ?? "."; - ProcessResult result = RunProcess("dotnet", ["sln", solutionPath, "list"], solutionDirectory, options.Logger, options.SdkQueryTimeout, options.CancellationToken); + ProcessResult result = RunProcess(DotNetCli, ["sln", solutionPath, "list"], solutionDirectory, options.Logger, options.SdkQueryTimeout, options.CancellationToken); if (result.ExitCode != 0) { throw new SolutionDocumentationGenerationException( $"Failed to list the projects of solution '{solutionPath}' (exit code {result.ExitCode}).\n{result.StandardError}"); @@ -127,10 +130,10 @@ private static List ReadSolutionProjects(string solutionPath, SolutionGe // The command prints a (localized) header followed by the project paths, relative to the solution. Keep // only lines that resolve to an existing project file — this is robust to the header and to localization. - if (line.EndsWith("proj", StringComparison.OrdinalIgnoreCase) is false) { continue; } + if (!line.EndsWith("proj", StringComparison.OrdinalIgnoreCase)) { continue; } string projectPath = Path.GetFullPath(Path.Combine(solutionDirectory, line)); - if (File.Exists(projectPath) is false) { continue; } + if (!File.Exists(projectPath)) { continue; } projects.Add(projectPath); } @@ -141,18 +144,14 @@ private static List ReadSolutionProjects(string solutionPath, SolutionGe internal static IReadOnlyList FilterProjects(IReadOnlyList projectPaths, SolutionGenerationOptions options) { List included = new(); - foreach (string projectPath in projectPaths) { - if (ShouldIncludeProject(projectPath, options)) { - included.Add(projectPath); - } - } + included.AddRange(projectPaths.Where(projectPath => ShouldIncludeProject(projectPath, options))); // An opt-in declared only in a shared build file (Directory.Build.props, an import) reads as absent in every // .csproj — per project, that is indistinguishable from a genuine absence, so it cannot be diagnosed above. The // one signature visible at this level is a solution where nothing opted in while the filter was active: name the // most likely cause instead of handing back an empty catalog in silence. A warning, not a failure — an empty // catalog is legitimate for a solution that documents no errors. - if (included.Count == 0 && projectPaths.Count > 0 && options.IncludeProjectsWithoutOptIn is false) { + if (included.Count == 0 && projectPaths.Count > 0 && !options.IncludeProjectsWithoutOptIn) { options.Logger.Warning( $"No project opted in to error documentation: '{options.OptInPropertyName}' was not set to a truthy value " + "in any project file. GenDoc reads this property literally from the .csproj, without MSBuild evaluation: a " + @@ -213,10 +212,10 @@ private static OptInReadResult ReadOptIn(string projectPath, string optInPropert } private static string ResolveTargetFrameworkMoniker(string projectPath, SolutionGenerationOptions options) { - if (string.IsNullOrWhiteSpace(options.TargetFramework) is false) { return options.TargetFramework!; } + if (!string.IsNullOrWhiteSpace(options.TargetFramework)) { return options.TargetFramework!; } string? single = ReadMsBuildProperty(projectPath, "TargetFramework"); - if (string.IsNullOrWhiteSpace(single) is false) { return single; } + if (!string.IsNullOrWhiteSpace(single)) { return single; } string? multi = ReadMsBuildProperty(projectPath, "TargetFrameworks"); if (string.IsNullOrWhiteSpace(multi)) { @@ -250,7 +249,7 @@ private static MsBuildPropertyRead ReadMsBuildPropertyDetailed(string projectPat List matches = document .Descendants() .Where(element => string.Equals(element.Name.LocalName, "PropertyGroup", StringComparison.OrdinalIgnoreCase)) - .Where(propertyGroup => IsInsideTarget(propertyGroup) is false) + .Where(propertyGroup => !IsInsideTarget(propertyGroup)) .SelectMany(propertyGroup => propertyGroup.Elements()) .Where(property => string.Equals(property.Name.LocalName, propertyName, StringComparison.OrdinalIgnoreCase)) .ToList(); @@ -301,16 +300,14 @@ private static bool IsInsideTarget(XElement propertyGroup) { private static void DotNetBuild(string solutionPath, SolutionGenerationOptions options) { List args = ["build", solutionPath, "-c", options.Configuration]; - if (string.IsNullOrWhiteSpace(options.TargetFramework) is false) { + if (!string.IsNullOrWhiteSpace(options.TargetFramework)) { args.Add("-f"); args.Add(options.TargetFramework); } - foreach (string additionalArgument in options.DotNetBuildAdditionalArguments) { - if (string.IsNullOrWhiteSpace(additionalArgument) is false) { args.Add(additionalArgument); } - } + args.AddRange(options.DotNetBuildAdditionalArguments.Where(additionalArgument => !string.IsNullOrWhiteSpace(additionalArgument))); - ProcessResult result = RunProcess("dotnet", args, Path.GetDirectoryName(solutionPath)!, options.Logger, options.BuildTimeout, options.CancellationToken); + ProcessResult result = RunProcess(DotNetCli, args, Path.GetDirectoryName(solutionPath)!, options.Logger, options.BuildTimeout, options.CancellationToken); if (result.ExitCode != 0) { throw new SolutionDocumentationGenerationException( @@ -322,13 +319,13 @@ private static void DotNetBuild(string solutionPath, SolutionGenerationOptions o // dotnet msbuild -getProperty:TargetPath -property:Configuration=Release -property:TargetFramework=net8.0 -nologo List args = ["msbuild", projectPath, $"-getProperty:{propertyName}", $"-property:Configuration={configuration}"]; - if (string.IsNullOrWhiteSpace(targetFramework) is false) { + if (!string.IsNullOrWhiteSpace(targetFramework)) { args.Add($"-property:TargetFramework={targetFramework}"); } args.Add("-nologo"); - ProcessResult result = RunProcess("dotnet", args, Path.GetDirectoryName(projectPath)!, options.Logger, options.SdkQueryTimeout, options.CancellationToken); + ProcessResult result = RunProcess(DotNetCli, args, Path.GetDirectoryName(projectPath)!, options.Logger, options.SdkQueryTimeout, options.CancellationToken); if (result.ExitCode != 0) { return null; @@ -339,7 +336,7 @@ private static void DotNetBuild(string solutionPath, SolutionGenerationOptions o string output = result.StandardOutput.Trim(); string line = output .Split(['\r', '\n'], StringSplitOptions.RemoveEmptyEntries) - .FirstOrDefault(l => l.IndexOf(propertyName, StringComparison.OrdinalIgnoreCase) >= 0) + .FirstOrDefault(l => l.Contains(propertyName, StringComparison.OrdinalIgnoreCase)) ?? output.Split(['\r', '\n'], StringSplitOptions.RemoveEmptyEntries).LastOrDefault() ?? string.Empty; @@ -361,11 +358,11 @@ private static string ParseMsBuildGetPropertyLine(string line, string propertyNa string afterName = line.Substring(idx + propertyName.Length).Trim(); - if (afterName.StartsWith("=", StringComparison.Ordinal)) { + if (afterName.StartsWith('=')) { return afterName.Substring(1).Trim(); } - if (afterName.StartsWith(":", StringComparison.Ordinal)) { + if (afterName.StartsWith(':')) { return afterName.Substring(1).Trim(); } @@ -409,7 +406,7 @@ private static ProcessResult RunProcess(string fileName, IReadOnlyList a }; bool started = process.Start(); - if (started is false) { throw new SolutionDocumentationGenerationException($"Failed to start process '{fileName}'."); } + if (!started) { throw new SolutionDocumentationGenerationException($"Failed to start process '{fileName}'."); } process.BeginOutputReadLine(); process.BeginErrorReadLine(); @@ -426,22 +423,20 @@ private static ProcessResult RunProcess(string fileName, IReadOnlyList a } }); - if (timeout is { } limit) { - if (process.WaitForExit((int)limit.TotalMilliseconds) is false) { - try { - process.Kill(entireProcessTree: true); - } catch { - // The process may already have exited between the timeout and the kill. - } + if (timeout is { } limit && !process.WaitForExit((int)limit.TotalMilliseconds)) { + try { + process.Kill(entireProcessTree: true); + } catch { + // The process may already have exited between the timeout and the kill. + } - process.WaitForExit(); + process.WaitForExit(); - // A cancellation that raced the timeout is reported as cancellation, not as a spurious timeout. - cancellationToken.ThrowIfCancellationRequested(); - logger.Error($"Process '{fileName}' timed out after {limit} and was killed."); + // A cancellation that raced the timeout is reported as cancellation, not as a spurious timeout. + cancellationToken.ThrowIfCancellationRequested(); + logger.Error($"Process '{fileName}' timed out after {limit} and was killed."); - return new ProcessResult(-1, stdout.ToString(), stderr.ToString()); - } + return new ProcessResult(-1, stdout.ToString(), stderr.ToString()); } // A final no-argument wait blocks until the async stdout/stderr handlers have flushed. @@ -533,8 +528,8 @@ internal static IReadOnlyList DeduplicateAcrossAssemblies(IR } private static string ResolveWorkerAssemblyPath(SolutionGenerationOptions options) { - if (string.IsNullOrWhiteSpace(options.WorkerAssemblyPath) is false) { - if (File.Exists(options.WorkerAssemblyPath) is false) { + if (!string.IsNullOrWhiteSpace(options.WorkerAssemblyPath)) { + if (!File.Exists(options.WorkerAssemblyPath)) { throw new SolutionDocumentationGenerationException($"The configured documentation worker was not found at '{options.WorkerAssemblyPath}'."); } @@ -579,7 +574,7 @@ private static ErrorDocumentationExtractionResult RunWorker(string workerAssembl args.Add(options.Culture.Name); } - ProcessResult result = RunProcess("dotnet", args, workerDirectory, options.Logger, options.WorkerTimeout, options.CancellationToken); + ProcessResult result = RunProcess(DotNetCli, args, workerDirectory, options.Logger, options.WorkerTimeout, options.CancellationToken); if (result.ExitCode != 0) { HandleFailure(options, $"The documentation worker failed (exit code {result.ExitCode}) for '{targetAssemblyPath}'.\n{result.StandardError}"); @@ -587,7 +582,7 @@ private static ErrorDocumentationExtractionResult RunWorker(string workerAssembl return new ErrorDocumentationExtractionResult([], []); } - if (File.Exists(outputPath) is false) { + if (!File.Exists(outputPath)) { HandleFailure(options, $"The documentation worker produced no output for '{targetAssemblyPath}'."); return new ErrorDocumentationExtractionResult([], []); diff --git a/FirstClassErrors.PropertyTests/ErrorDescriptionPropertyTests.cs b/FirstClassErrors.PropertyTests/ErrorDescriptionPropertyTests.cs index 031d1d9..5be497a 100644 --- a/FirstClassErrors.PropertyTests/ErrorDescriptionPropertyTests.cs +++ b/FirstClassErrors.PropertyTests/ErrorDescriptionPropertyTests.cs @@ -76,13 +76,13 @@ from detail in optionalDetail [Fact(DisplayName = "A blank short message is rejected with an ArgumentException.")] public void BlankShortMessageIsRejected() { - Prop.ForAll(Generators.Blank().ToArbitrary(), blank => Expect.Throws(() => new ErrorDescription(blank, "diagnostic"))) + Prop.ForAll(Generators.Blank().ToArbitrary(), blank => Expect.Throws(() => _ = new ErrorDescription(blank, "diagnostic"))) .QuickCheckThrowOnFailure(); } [Fact(DisplayName = "A blank diagnostic message is rejected with an ArgumentException.")] public void BlankDiagnosticMessageIsRejected() { - Prop.ForAll(Generators.Blank().ToArbitrary(), blank => Expect.Throws(() => new ErrorDescription("short", blank))) + Prop.ForAll(Generators.Blank().ToArbitrary(), blank => Expect.Throws(() => _ = new ErrorDescription("short", blank))) .QuickCheckThrowOnFailure(); } diff --git a/FirstClassErrors.Testing/OutcomeAssertions.cs b/FirstClassErrors.Testing/OutcomeAssertions.cs index 5a86299..773bdd9 100644 --- a/FirstClassErrors.Testing/OutcomeAssertions.cs +++ b/FirstClassErrors.Testing/OutcomeAssertions.cs @@ -1,3 +1,5 @@ +using System.Diagnostics.CodeAnalysis; + namespace FirstClassErrors.Testing; /// @@ -16,6 +18,11 @@ public static class OutcomeAssertions { /// /// Thrown when is null. /// Thrown when the outcome is a failure. + [SuppressMessage("Minor Code Smell", "S4136:Method overloads should be grouped together", + Justification = + "Grouped by receiver type (Outcome then Outcome), each block pairing ShouldSucceed/ShouldFail, " + + "parallel to OutcomeTaskExtensions. Grouping by assertion name would interleave the non-generic and " + + "generic surfaces.")] public static void ShouldSucceed(this Outcome outcome) { if (outcome is null) { throw new ArgumentNullException(nameof(outcome)); } @@ -30,6 +37,8 @@ public static void ShouldSucceed(this Outcome outcome) { /// An over the failing error. /// Thrown when is null. /// Thrown when the outcome is a success. + [SuppressMessage("Minor Code Smell", "S4136:Method overloads should be grouped together", + Justification = "Grouped by receiver type, not by assertion name; see the note on ShouldSucceed.")] public static ErrorAssertion ShouldFail(this Outcome outcome) { if (outcome is null) { throw new ArgumentNullException(nameof(outcome)); } diff --git a/FirstClassErrors.UnitTests/ErrorContextTests.cs b/FirstClassErrors.UnitTests/ErrorContextTests.cs index 198c788..495d7f3 100644 --- a/FirstClassErrors.UnitTests/ErrorContextTests.cs +++ b/FirstClassErrors.UnitTests/ErrorContextTests.cs @@ -146,7 +146,7 @@ public void AnErrorContextCanBeConvertedToANameBasedDictionary() { IReadOnlyDictionary dict = context.ToNameDictionary(); // Verify - string[] expectedKeys = new[] { "DealId", "UserId" }.OrderBy(x => x).ToArray(); + string[] expectedKeys = ["DealId", "UserId"]; Check.That(dict.Keys.OrderBy(x => x)).ContainsExactly(expectedKeys); Check.That(dict["DealId"]).IsEqualTo(123); diff --git a/FirstClassErrors.Usage/Infrastructure/Adapters/BankTransactionFileValidator.cs b/FirstClassErrors.Usage/Infrastructure/Adapters/BankTransactionFileValidator.cs index 9639f54..c110f7d 100644 --- a/FirstClassErrors.Usage/Infrastructure/Adapters/BankTransactionFileValidator.cs +++ b/FirstClassErrors.Usage/Infrastructure/Adapters/BankTransactionFileValidator.cs @@ -1,4 +1,6 @@ -namespace FirstClassErrors.Usage.Infrastructure.Adapters; +using System.Diagnostics.CodeAnalysis; + +namespace FirstClassErrors.Usage.Infrastructure.Adapters; /// /// Validates bank transaction files to ensure compliance with predefined business rules and data integrity @@ -8,4 +10,6 @@ /// This class is designed to detect and report issues in bank transaction files, such as transactions occurring /// outside the statement period. /// +[SuppressMessage("Minor Code Smell", "S2094:Classes should not be empty", + Justification = "An intentional minimal marker: it only anchors the bank-transaction-file error examples through [ProvidesErrorsFor(nameof(...))] and carries no behaviour of its own.")] public sealed class BankTransactionFileValidator { } \ No newline at end of file diff --git a/FirstClassErrors.Usage/Infrastructure/Adapters/ExchangeRateProvider.cs b/FirstClassErrors.Usage/Infrastructure/Adapters/ExchangeRateProvider.cs index 34d5822..022e886 100644 --- a/FirstClassErrors.Usage/Infrastructure/Adapters/ExchangeRateProvider.cs +++ b/FirstClassErrors.Usage/Infrastructure/Adapters/ExchangeRateProvider.cs @@ -1,3 +1,5 @@ +using System.Diagnostics.CodeAnalysis; + namespace FirstClassErrors.Usage.Infrastructure.Adapters; /// @@ -6,4 +8,6 @@ namespace FirstClassErrors.Usage.Infrastructure.Adapters; /// /// A minimal marker used only to anchor the exchange-rate error examples. /// +[SuppressMessage("Minor Code Smell", "S2094:Classes should not be empty", + Justification = "An intentional minimal marker: it only anchors the exchange-rate error examples through [ProvidesErrorsFor(nameof(...))] and carries no behaviour of its own.")] public sealed class ExchangeRateProvider { } diff --git a/FirstClassErrors.Usage/Infrastructure/Adapters/ExchangeRateProviderError.cs b/FirstClassErrors.Usage/Infrastructure/Adapters/ExchangeRateProviderError.cs index 185719f..9803526 100644 --- a/FirstClassErrors.Usage/Infrastructure/Adapters/ExchangeRateProviderError.cs +++ b/FirstClassErrors.Usage/Infrastructure/Adapters/ExchangeRateProviderError.cs @@ -76,6 +76,8 @@ private static ErrorDocumentation UnsupportedCurrencyPairDocumentation() { #region Nested types declarations + [System.Diagnostics.CodeAnalysis.SuppressMessage("Critical Code Smell", "S3218:Inner class members should not shadow outer class \"static\" members", + Justification = "Each error-code constant deliberately mirrors the name of its factory method; both are always qualified (Code.X versus the X(...) call), so there is no real ambiguity.")] private static class Code { #region Statics members declarations diff --git a/FirstClassErrors.Usage/Infrastructure/Adapters/NonCompliantBankTransactionFileError.cs b/FirstClassErrors.Usage/Infrastructure/Adapters/NonCompliantBankTransactionFileError.cs index 8a3f2f6..ec36977 100644 --- a/FirstClassErrors.Usage/Infrastructure/Adapters/NonCompliantBankTransactionFileError.cs +++ b/FirstClassErrors.Usage/Infrastructure/Adapters/NonCompliantBankTransactionFileError.cs @@ -84,6 +84,8 @@ private static ErrorDocumentation StatementTotalAmountMismatchDocumentation() { #region Nested types declarations + [System.Diagnostics.CodeAnalysis.SuppressMessage("Critical Code Smell", "S3218:Inner class members should not shadow outer class \"static\" members", + Justification = "Each error-code constant deliberately mirrors the name of its factory method; both are always qualified (Code.X versus the X(...) call), so there is no real ambiguity.")] private static class Code { // ReSharper disable MemberHidesStaticFromOuterClass diff --git a/FirstClassErrors.Usage/Infrastructure/Adapters/StatementUploadEndpoint.cs b/FirstClassErrors.Usage/Infrastructure/Adapters/StatementUploadEndpoint.cs index 32f12ce..5e7440a 100644 --- a/FirstClassErrors.Usage/Infrastructure/Adapters/StatementUploadEndpoint.cs +++ b/FirstClassErrors.Usage/Infrastructure/Adapters/StatementUploadEndpoint.cs @@ -1,3 +1,5 @@ +using System.Diagnostics.CodeAnalysis; + namespace FirstClassErrors.Usage.Infrastructure.Adapters; /// @@ -6,4 +8,6 @@ namespace FirstClassErrors.Usage.Infrastructure.Adapters; /// /// A minimal marker used only to anchor the statement-upload error examples. /// +[SuppressMessage("Minor Code Smell", "S2094:Classes should not be empty", + Justification = "An intentional minimal marker: it only anchors the statement-upload error examples through [ProvidesErrorsFor(nameof(...))] and carries no behaviour of its own.")] public sealed class StatementUploadEndpoint { } diff --git a/FirstClassErrors.Usage/Infrastructure/Adapters/StatementUploadEndpointError.cs b/FirstClassErrors.Usage/Infrastructure/Adapters/StatementUploadEndpointError.cs index 7c7160d..93db821 100644 --- a/FirstClassErrors.Usage/Infrastructure/Adapters/StatementUploadEndpointError.cs +++ b/FirstClassErrors.Usage/Infrastructure/Adapters/StatementUploadEndpointError.cs @@ -68,6 +68,8 @@ private static ErrorDocumentation RateLimitedDocumentation() { #region Nested types declarations + [System.Diagnostics.CodeAnalysis.SuppressMessage("Critical Code Smell", "S3218:Inner class members should not shadow outer class \"static\" members", + Justification = "Each error-code constant deliberately mirrors the name of its factory method; both are always qualified (Code.X versus the X(...) call), so there is no real ambiguity.")] private static class Code { #region Statics members declarations diff --git a/FirstClassErrors.Usage/Model/Amount.cs b/FirstClassErrors.Usage/Model/Amount.cs index 38acd51..cc87b4b 100644 --- a/FirstClassErrors.Usage/Model/Amount.cs +++ b/FirstClassErrors.Usage/Model/Amount.cs @@ -1,6 +1,7 @@ #region Usings declarations using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using FirstClassErrors.Usage.Utils; @@ -17,6 +18,11 @@ namespace FirstClassErrors.Usage.Model; /// This type is not intended to be a full or production-ready Value Object implementation. /// [DebuggerDisplay("{ToString()}")] +[SuppressMessage("Minor Code Smell", "S1210:\"Equals\" and the comparison operators should be overridden when implementing \"IComparable\"", + Justification = + "Amount is an intentionally minimal illustrative model (see its remarks), not a production value object. " + + "CompareTo exists to demonstrate the documentation flow; the full set of comparison operators is out of " + + "scope for the example and would add untested surface to a type the tests only exercise indirectly.")] public sealed class Amount : IEquatable, IComparable { #region Constructors declarations diff --git a/FirstClassErrors.Usage/Model/DeltaTemperature.cs b/FirstClassErrors.Usage/Model/DeltaTemperature.cs deleted file mode 100644 index bbb3a78..0000000 --- a/FirstClassErrors.Usage/Model/DeltaTemperature.cs +++ /dev/null @@ -1,5 +0,0 @@ -namespace FirstClassErrors.Usage.Model { - - internal class DeltaTemperature { } - -} \ No newline at end of file diff --git a/FirstClassErrors.Usage/Model/InvalidAmountOperationError.cs b/FirstClassErrors.Usage/Model/InvalidAmountOperationError.cs index 7b58d84..9be5349 100644 --- a/FirstClassErrors.Usage/Model/InvalidAmountOperationError.cs +++ b/FirstClassErrors.Usage/Model/InvalidAmountOperationError.cs @@ -46,6 +46,8 @@ private static ErrorDocumentation CurrencyMismatchDocumentation() { #region Nested types declarations + [System.Diagnostics.CodeAnalysis.SuppressMessage("Critical Code Smell", "S3218:Inner class members should not shadow outer class \"static\" members", + Justification = "Each error-code constant deliberately mirrors the name of its factory method; both are always qualified (Code.X versus the X(...) call), so there is no real ambiguity.")] private static class Code { #region Statics members declarations diff --git a/FirstClassErrors.Usage/Model/InvalidMoneyTransferError.cs b/FirstClassErrors.Usage/Model/InvalidMoneyTransferError.cs index 554cf8d..857000c 100644 --- a/FirstClassErrors.Usage/Model/InvalidMoneyTransferError.cs +++ b/FirstClassErrors.Usage/Model/InvalidMoneyTransferError.cs @@ -70,6 +70,8 @@ private static ErrorDocumentation InvalidDocumentation() { #region Nested types declarations + [System.Diagnostics.CodeAnalysis.SuppressMessage("Critical Code Smell", "S3218:Inner class members should not shadow outer class \"static\" members", + Justification = "Each error-code constant deliberately mirrors the name of its factory method; both are always qualified (Code.X versus the X(...) call), so there is no real ambiguity.")] private static class Code { #region Statics members declarations diff --git a/FirstClassErrors.Usage/Model/MoneyTransfer.cs b/FirstClassErrors.Usage/Model/MoneyTransfer.cs index 1d9e12b..d15fda2 100644 --- a/FirstClassErrors.Usage/Model/MoneyTransfer.cs +++ b/FirstClassErrors.Usage/Model/MoneyTransfer.cs @@ -1,3 +1,5 @@ +using System.Diagnostics.CodeAnalysis; + namespace FirstClassErrors.Usage.Model; /// @@ -6,4 +8,6 @@ namespace FirstClassErrors.Usage.Model; /// /// A minimal, intentionally simplified domain concept used only to anchor the money-transfer error examples. /// +[SuppressMessage("Minor Code Smell", "S2094:Classes should not be empty", + Justification = "An intentional minimal marker: it only anchors the money-transfer error examples through [ProvidesErrorsFor(nameof(...))] and carries no behaviour of its own.")] public sealed class MoneyTransfer { } diff --git a/FirstClassErrors.Usage/Utils/DocumentationFormatter.cs b/FirstClassErrors.Usage/Utils/DocumentationFormatter.cs index 93a6073..d4363e7 100644 --- a/FirstClassErrors.Usage/Utils/DocumentationFormatter.cs +++ b/FirstClassErrors.Usage/Utils/DocumentationFormatter.cs @@ -58,7 +58,7 @@ private static string FormatDynamic(byte value) { private static string FormatDynamic(decimal value) { string formattedValue = value.ToString(DocumentationCulture); - if (formattedValue.Contains(".")) { + if (formattedValue.Contains('.')) { formattedValue = formattedValue.TrimEnd('0').TrimEnd('.'); } diff --git a/FirstClassErrors/ErrorContextKey.cs b/FirstClassErrors/ErrorContextKey.cs index 728c186..1a4856d 100644 --- a/FirstClassErrors/ErrorContextKey.cs +++ b/FirstClassErrors/ErrorContextKey.cs @@ -1,6 +1,7 @@ #region Usings declarations using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; #endregion @@ -40,6 +41,12 @@ namespace FirstClassErrors; /// /// [DebuggerDisplay("{ToString()}")] +[SuppressMessage("Major Code Smell", "S4035:Classes implementing \"IEquatable\" should be sealed", + Justification = + "ErrorContextKey is abstract by design — the base of the sealed ErrorContextKey — so it cannot " + + "itself be sealed. Identity is defined by Name for the whole hierarchy: Equals compares Name after a " + + "type-agnostic 'is ErrorContextKey' check, so the single sealed subclass cannot break the symmetry of " + + "the IEquatable contract.")] public abstract class ErrorContextKey : IEquatable { #region Statics members declarations diff --git a/FirstClassErrors/ErrorDescription.cs b/FirstClassErrors/ErrorDescription.cs index 6be39e1..6e79420 100644 --- a/FirstClassErrors/ErrorDescription.cs +++ b/FirstClassErrors/ErrorDescription.cs @@ -43,7 +43,9 @@ public ErrorDescription(string shortMessage, string diagnosticMessage, string? d ShortMessage = shortMessage.Trim(); DiagnosticMessage = diagnosticMessage.Trim(); - DetailedMessage = string.IsNullOrWhiteSpace(detailedMessage) ? null : detailedMessage?.Trim(); + // In this branch IsNullOrWhiteSpace(detailedMessage) is false, so detailedMessage is non-null; the null-forgiving + // '!' states that for the netstandard2.0 compiler, which does not carry the [NotNullWhen(false)] flow annotation. + DetailedMessage = string.IsNullOrWhiteSpace(detailedMessage) ? null : detailedMessage!.Trim(); } #endregion diff --git a/FirstClassErrors/ErrorDocumentationBuilder.cs b/FirstClassErrors/ErrorDocumentationBuilder.cs index cf1dada..a383afa 100644 --- a/FirstClassErrors/ErrorDocumentationBuilder.cs +++ b/FirstClassErrors/ErrorDocumentationBuilder.cs @@ -84,10 +84,10 @@ public IErrorDescriptionStage WithTitle(string title) { return this; } - public IErrorRuleStage WithDescription(string explanation) { - if (explanation is null) { throw new ArgumentNullException(nameof(explanation)); } + public IErrorRuleStage WithDescription(string description) { + if (description is null) { throw new ArgumentNullException(nameof(description)); } - _doc.Explanation = explanation.Trim(); + _doc.Explanation = description.Trim(); return this; } @@ -164,9 +164,9 @@ private IEnumerable BuildExamples(TError[] errors) /// IErrorExamplesOrDiagnosticsStage IErrorDiagnosticsStage.WithDiagnostic(string cause, ErrorOrigin type, string analysisLead) { - _diagnostics.Add(new ErrorDiagnostic(cause, type, analysisLead)); - - return this; + // The first diagnostic ('WithDiagnostic') and any subsequent one ('AndDiagnostic') read differently at the + // call site but do the same thing; delegate so the two stay in step. + return AndDiagnostic(cause, type, analysisLead); } } \ No newline at end of file diff --git a/FirstClassErrors/GenDoc/AssemblyErrorDocumentationReader.cs b/FirstClassErrors/GenDoc/AssemblyErrorDocumentationReader.cs index f279961..9836538 100644 --- a/FirstClassErrors/GenDoc/AssemblyErrorDocumentationReader.cs +++ b/FirstClassErrors/GenDoc/AssemblyErrorDocumentationReader.cs @@ -1,5 +1,6 @@ #region Usings declarations +using System.Diagnostics.CodeAnalysis; using System.Reflection; using System.Resources; @@ -45,7 +46,7 @@ public static ErrorDocumentationExtractionResult GetErrorDocumentationFrom(Assem List failures = new(); foreach (Type type in GetLoadableTypes(assembly, failures)) { - if (IsExtractableType(type) is false) { continue; } + if (!IsExtractableType(type)) { continue; } ExtractFromType(type, documentation, failures); } @@ -117,6 +118,11 @@ private static bool IsExtractableType(Type type) { return type is { IsClass: true, IsGenericTypeDefinition: false }; } + [SuppressMessage("Major Code Smell", "S3011:Reflection should not be used to increase accessibility of classes, methods, or fields", + Justification = + "Discovering [DocumentedBy] documentation factories regardless of their declared accessibility is the " + + "feature: an author may keep a factory internal or private. BindingFlags.NonPublic is intentional here, " + + "not an accidental accessibility bypass.")] private static void ExtractFromType(Type type, List documentation, List failures) { ProvidesErrorsForAttribute? providesErrorsFor; try { @@ -168,6 +174,10 @@ private static void ExtractFromType(Type type, List document } } + [SuppressMessage("Major Code Smell", "S3011:Reflection should not be used to increase accessibility of classes, methods, or fields", + Justification = + "The [DocumentedBy] factory it resolves may be declared non-public on purpose; scanning non-public static " + + "methods is the feature, not an accidental accessibility bypass.")] private static MethodInfo? ResolveDocumentationMethod(Type type, string methodName, List failures) { // Resolve by hand rather than Type.GetMethod(name, flags), which throws AmbiguousMatchException when the name // is overloaded. We only ever invoke a parameterless static method with no type arguments, so we filter to that diff --git a/FirstClassErrors/OutcomeTaskExtensions.cs b/FirstClassErrors/OutcomeTaskExtensions.cs index 63df786..7a57ae8 100644 --- a/FirstClassErrors/OutcomeTaskExtensions.cs +++ b/FirstClassErrors/OutcomeTaskExtensions.cs @@ -1,3 +1,5 @@ +using System.Diagnostics.CodeAnalysis; + namespace FirstClassErrors; /// @@ -39,6 +41,11 @@ public static class OutcomeTaskExtensions { /// If the preceding is not successful, the continuation function is not invoked, and its error /// is propagated as a failed . /// + [SuppressMessage("Minor Code Smell", "S4136:Method overloads should be grouped together", + Justification = + "Overloads are organized by receiver type — Task first, then Task> — each block " + + "covering Then/Recover/Finally/To (see the section headers). This receiver-first grouping is deliberate; " + + "grouping by operation would interleave the non-generic and generic surfaces.")] public static Task> Then(this Task task, Func> next) where TResult : notnull { if (task is null) { throw new ArgumentNullException(nameof(task)); } @@ -168,6 +175,8 @@ async Task Core() { /// /// Thrown if the is null. /// + [SuppressMessage("Minor Code Smell", "S4136:Method overloads should be grouped together", + Justification = "Overloads are grouped by receiver type, not by operation; see the note on Then.")] public static Task Recover(this Task task, Func fallback) { if (task is null) { throw new ArgumentNullException(nameof(task)); } if (fallback is null) { throw new ArgumentNullException(nameof(fallback)); } @@ -243,6 +252,8 @@ async Task Core() { /// /// Thrown if the is null. /// + [SuppressMessage("Minor Code Smell", "S4136:Method overloads should be grouped together", + Justification = "Overloads are grouped by receiver type, not by operation; see the note on Then.")] public static Task Finally(this Task task, Func onSuccess, Func onFailure) { diff --git a/tools/commit-lint/lint-commit-message.sh b/tools/commit-lint/lint-commit-message.sh index b9f4712..e77a2ae 100755 --- a/tools/commit-lint/lint-commit-message.sh +++ b/tools/commit-lint/lint-commit-message.sh @@ -33,6 +33,7 @@ MAX=72 strict=0 case "${1:-}" in --ci|--strict) strict=1; shift ;; + *) ;; # any other first argument is the message file / '-', handled below esac # --- read the message --------------------------------------------------------- @@ -57,6 +58,7 @@ subject="$(printf '%s\n' "$msg" | sed -n '1p' | sed 's/[[:space:]]*$//')" # also filters them out with --no-merges). case "$subject" in 'Merge '*) exit 0 ;; + *) ;; # not a merge commit: fall through to validation esac # Autosquash placeholders are rewritten by a later `git rebase --autosquash`, so # the local hook lets them through. CI (--ci) rejects them instead: this repo @@ -70,6 +72,7 @@ case "$subject" in printf 'commit-lint: autosquash placeholder must be squashed away before merge: %s\n' "$subject" >&2 exit 1 ;; + *) ;; # not an autosquash placeholder: fall through to validation esac errors=0 @@ -78,6 +81,7 @@ err() { errmsgs="${errmsgs} - ${1} " errors=$((errors + 1)) + return 0 } # --- header: presence --------------------------------------------------------- @@ -124,12 +128,14 @@ else desc_start="$(printf '%s' "$subject" | sed -E 's/^[^:]*: ?//' | cut -c1)" case "$desc_start" in [A-Z]) err "the description must start with a lowercase letter (imperative: 'add', not 'Add'/'Added')" ;; + *) ;; # already lowercase (or non-letter): nothing to report esac fi # trailing period case "$subject" in *.) err "the header must not end with a period" ;; + *) ;; # no trailing period: nothing to report esac # scope order / duplicates (only meaningful when the group is well-formed) @@ -152,7 +158,7 @@ fi # --- breaking-change double signal -------------------------------------------- prefix="${subject%%: *}" has_bang=0 -case "$prefix" in *!) has_bang=1 ;; esac +case "$prefix" in *!) has_bang=1 ;; *) ;; esac has_breaking=0 if printf '%s\n' "$msg" | grep -Eq '^BREAKING CHANGE: '; then