From 568ed79ba884dd729aa5783926a66c9c5137c607 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Thu, 20 Apr 2023 17:06:54 +0300 Subject: [PATCH] Fix PackageReadmeFile and apply analyzers (#305) --- src/Directory.Build.props | 4 +++- .../ApiApprovalTests.cs | 2 +- .../GraphQLParser.approved.txt | 2 +- .../Benchmarks/BenchmarkBase.cs | 2 +- .../Benchmarks/ParserBenchmark.cs | 4 ++-- .../GraphQLParser.Benchmarks.csproj | 2 +- .../GraphQLParser.Tests.csproj | 3 ++- src/GraphQLParser.Tests/ParserTests.Throw.cs | 5 +++-- src/GraphQLParser.Tests/ParserTests.cs | 2 +- .../Values/GraphQLIntValueTests.cs | 21 ++++++++++--------- .../Values/GraphQLNameTests.cs | 4 ++-- .../Visitors/ASTVisitorTests.cs | 6 +++--- .../Visitors/GraphQLAstVisitorTests.cs | 7 +++---- .../Visitors/SDLPrinterFromParsedTextTests.cs | 2 +- .../Visitors/SDLPrinterSkipDirectivesTests.cs | 2 +- .../SDLPrinterVerticalIndentationTests.cs | 2 +- .../AST/Enums/DirectiveLocation.cs | 2 ++ .../Exceptions/GraphQLParserException.cs | 9 ++++---- .../Extensions/ASTNodeExtensions.cs | 9 ++++++-- src/GraphQLParser/TokenKind.cs | 4 ++++ src/GraphQLParser/Visitors/ASTVisitor.cs | 2 ++ src/GraphQLParser/Visitors/SDLPrinter.cs | 4 ++-- 22 files changed, 59 insertions(+), 41 deletions(-) diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 5c096baf..87a78b19 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -4,6 +4,7 @@ 8.0.0-preview latest true + $(NoWarn);CA1707 CS8766 true Marek Magdziak @@ -19,9 +20,10 @@ True true + Recommended enable enable - README.md + README.md true ..\graphql.snk diff --git a/src/GraphQLParser.ApiTests/ApiApprovalTests.cs b/src/GraphQLParser.ApiTests/ApiApprovalTests.cs index f8dc6a11..092376c4 100644 --- a/src/GraphQLParser.ApiTests/ApiApprovalTests.cs +++ b/src/GraphQLParser.ApiTests/ApiApprovalTests.cs @@ -27,7 +27,7 @@ public void Public_Api_Should_Not_Change_Inadvertently(Type type) string[] tfms = project.Descendants("TargetFrameworks").Union(project.Descendants("TargetFramework")).First().Value.Split(";", StringSplitOptions.RemoveEmptyEntries); // There may be old stuff from earlier builds like net45, netcoreapp3.0, etc. so filter it out - string[] actualTfmDirs = Directory.GetDirectories(buildDir).Where(dir => tfms.Any(tfm => dir.EndsWith(tfm))).ToArray(); + string[] actualTfmDirs = Directory.GetDirectories(buildDir).Where(dir => tfms.Any(tfm => dir.EndsWith(tfm, StringComparison.InvariantCulture))).ToArray(); Debug.Assert(actualTfmDirs.Length > 0, $"Directory '{buildDir}' doesn't contain subdirectories matching {string.Join(";", tfms)}"); (string tfm, string content)[] publicApi = actualTfmDirs.Select(tfmDir => (new DirectoryInfo(tfmDir).Name.Replace(".", ""), Assembly.LoadFile(Path.Combine(tfmDir, projectName + ".dll")).GeneratePublicApi(new ApiGeneratorOptions diff --git a/src/GraphQLParser.ApiTests/GraphQLParser.approved.txt b/src/GraphQLParser.ApiTests/GraphQLParser.approved.txt index 143b5c62..8e3b9485 100644 --- a/src/GraphQLParser.ApiTests/GraphQLParser.approved.txt +++ b/src/GraphQLParser.ApiTests/GraphQLParser.approved.txt @@ -889,7 +889,7 @@ namespace GraphQLParser.Visitors public DefaultPrintContext(System.IO.TextWriter writer) { } public int IndentLevel { get; set; } public bool IndentPrinted { get; set; } - [System.Obsolete] + [System.Obsolete("Use LastVisitedNode instead")] public bool LastDefinitionPrinted { get; set; } public GraphQLParser.AST.ASTNode? LastVisitedNode { get; set; } public bool NewLinePrinted { get; set; } diff --git a/src/GraphQLParser.Benchmarks/Benchmarks/BenchmarkBase.cs b/src/GraphQLParser.Benchmarks/Benchmarks/BenchmarkBase.cs index 909366f3..0672fc5a 100644 --- a/src/GraphQLParser.Benchmarks/Benchmarks/BenchmarkBase.cs +++ b/src/GraphQLParser.Benchmarks/Benchmarks/BenchmarkBase.cs @@ -35,7 +35,7 @@ public string GetQueryByName(string name) "params" => _params, "variables" => _variables, "github" => _github, - _ => throw new System.Exception(name) + _ => throw new ArgumentException(name, nameof(name)) }; } diff --git a/src/GraphQLParser.Benchmarks/Benchmarks/ParserBenchmark.cs b/src/GraphQLParser.Benchmarks/Benchmarks/ParserBenchmark.cs index 6bf05766..3a170a3c 100644 --- a/src/GraphQLParser.Benchmarks/Benchmarks/ParserBenchmark.cs +++ b/src/GraphQLParser.Benchmarks/Benchmarks/ParserBenchmark.cs @@ -15,7 +15,7 @@ namespace GraphQLParser.Benchmarks; //[RPlotExporter, CsvMeasurementsExporter] public class ParserBenchmark : BenchmarkBase { - private class Config : ManualConfig + private sealed class Config : ManualConfig { public Config() { @@ -23,7 +23,7 @@ public Config() Orderer = new ParserOrderer(); } - private class ParserOrderer : IOrderer + private sealed class ParserOrderer : IOrderer { private static int GetOrder(object o) => o switch { diff --git a/src/GraphQLParser.Benchmarks/GraphQLParser.Benchmarks.csproj b/src/GraphQLParser.Benchmarks/GraphQLParser.Benchmarks.csproj index 31d3aa32..9395013e 100644 --- a/src/GraphQLParser.Benchmarks/GraphQLParser.Benchmarks.csproj +++ b/src/GraphQLParser.Benchmarks/GraphQLParser.Benchmarks.csproj @@ -4,7 +4,7 @@ net7 Exe false - $(NoWarn);1591 + $(NoWarn);1591;CA1822 diff --git a/src/GraphQLParser.Tests/GraphQLParser.Tests.csproj b/src/GraphQLParser.Tests/GraphQLParser.Tests.csproj index a5239c52..1f43930c 100644 --- a/src/GraphQLParser.Tests/GraphQLParser.Tests.csproj +++ b/src/GraphQLParser.Tests/GraphQLParser.Tests.csproj @@ -4,7 +4,8 @@ false disable - $(NoWarn);1591;IDE0008;IDE0022;IDE0058;IDE1006 + $(NoWarn);1591;CA2012;IDE0008;IDE0022;IDE0058;IDE1006 + false diff --git a/src/GraphQLParser.Tests/ParserTests.Throw.cs b/src/GraphQLParser.Tests/ParserTests.Throw.cs index 616da69e..de462cd6 100644 --- a/src/GraphQLParser.Tests/ParserTests.Throw.cs +++ b/src/GraphQLParser.Tests/ParserTests.Throw.cs @@ -1,3 +1,4 @@ +using System.Globalization; using GraphQLParser.Exceptions; namespace GraphQLParser.Tests; @@ -216,7 +217,7 @@ public void Should_Throw_GraphQLSyntaxErrorException(int number, string text, st { var ex = Should.Throw(() => text.Parse()); ex.Message.ShouldContain(ex.Description); - ex.Description.ShouldBe(description, number.ToString()); + ex.Description.ShouldBe(description, number.ToString(CultureInfo.InvariantCulture)); ex.Location.Line.ShouldBe(line); ex.Location.Column.ShouldBe(column); } @@ -261,7 +262,7 @@ public void Should_Throw_On_Empty_Types_With_Braces(int number, string text, str { var ex = Should.Throw(() => text.Parse()); ex.Message.ShouldContain(ex.Description); - ex.Description.ShouldBe(description, number.ToString()); + ex.Description.ShouldBe(description, number.ToString(CultureInfo.InvariantCulture)); ex.Location.Line.ShouldBe(line); ex.Location.Column.ShouldBe(column); } diff --git a/src/GraphQLParser.Tests/ParserTests.cs b/src/GraphQLParser.Tests/ParserTests.cs index f9de4ca6..f883240b 100644 --- a/src/GraphQLParser.Tests/ParserTests.cs +++ b/src/GraphQLParser.Tests/ParserTests.cs @@ -478,7 +478,7 @@ public void Comments_on_Variable_Should_Read_Correctly(IgnoreOptions options) //[InlineData(IgnoreOptions.Comments)] [InlineData(IgnoreOptions.Locations)] //[InlineData(IgnoreOptions.All)] - public void Comments_On_SelectionSet_Should_Read_Correctly(IgnoreOptions options) + public void Comments_On_Fields_Should_Read_Correctly(IgnoreOptions options) { var document = @" query { diff --git a/src/GraphQLParser.Tests/Values/GraphQLIntValueTests.cs b/src/GraphQLParser.Tests/Values/GraphQLIntValueTests.cs index 60fd1e69..f7aa6322 100644 --- a/src/GraphQLParser.Tests/Values/GraphQLIntValueTests.cs +++ b/src/GraphQLParser.Tests/Values/GraphQLIntValueTests.cs @@ -1,3 +1,4 @@ +using System.Globalization; using System.Numerics; namespace GraphQLParser.Tests; @@ -5,7 +6,7 @@ namespace GraphQLParser.Tests; public class GraphQLIntValueTests { [Fact] - public void Int() + public void IntValue_From_Int() { var value = new GraphQLIntValue(1234567); value.Value.Length.ShouldBe(7); @@ -13,7 +14,7 @@ public void Int() } [Fact] - public void Byte() + public void IntValue_From_Byte() { var value = new GraphQLIntValue((byte)42); value.Value.Length.ShouldBe(2); @@ -21,7 +22,7 @@ public void Byte() } [Fact] - public void Sbyte() + public void IntValue_From_Sbyte() { var value = new GraphQLIntValue((sbyte)-10); value.Value.Length.ShouldBe(3); @@ -29,7 +30,7 @@ public void Sbyte() } [Fact] - public void Short() + public void IntValue_From_Short() { var value = new GraphQLIntValue((short)-300); value.Value.Length.ShouldBe(4); @@ -37,7 +38,7 @@ public void Short() } [Fact] - public void Ushort() + public void IntValue_From_Ushort() { var value = new GraphQLIntValue((ushort)60000); value.Value.Length.ShouldBe(5); @@ -45,7 +46,7 @@ public void Ushort() } [Fact] - public void Uint() + public void IntValue_From_Uint() { var value = new GraphQLIntValue(2247483647U); value.Value.Length.ShouldBe(10); @@ -53,7 +54,7 @@ public void Uint() } [Fact] - public void Long() + public void IntValue_From_Long() { var value = new GraphQLIntValue(-60001L); value.Value.Length.ShouldBe(6); @@ -61,7 +62,7 @@ public void Long() } [Fact] - public void Ulong() + public void IntValue_From_Ulong() { var value = new GraphQLIntValue(9223372036854775808UL); value.Value.Length.ShouldBe(19); @@ -69,9 +70,9 @@ public void Ulong() } [Fact] - public void BigInt() + public void IntValue_From_BigInt() { - var value = new GraphQLIntValue(BigInteger.Parse("7922816251426433759354395033579228162514264337593543950335")); + var value = new GraphQLIntValue(BigInteger.Parse("7922816251426433759354395033579228162514264337593543950335", CultureInfo.InvariantCulture)); value.Value.Length.ShouldBe(58); value.Value.ShouldBe("7922816251426433759354395033579228162514264337593543950335"); } diff --git a/src/GraphQLParser.Tests/Values/GraphQLNameTests.cs b/src/GraphQLParser.Tests/Values/GraphQLNameTests.cs index 7f3cd324..9bacd01a 100644 --- a/src/GraphQLParser.Tests/Values/GraphQLNameTests.cs +++ b/src/GraphQLParser.Tests/Values/GraphQLNameTests.cs @@ -114,7 +114,7 @@ public void GraphQLName_Explicit_Cast() ((string)nameNull).ShouldBeNull(); } - private ROM FuncROM(ROM r) => r; + private static ROM FuncROM(ROM r) => r; - private string FuncString(string s) => s; + private static string FuncString(string s) => s; } diff --git a/src/GraphQLParser.Tests/Visitors/ASTVisitorTests.cs b/src/GraphQLParser.Tests/Visitors/ASTVisitorTests.cs index 339c6106..1abaea09 100644 --- a/src/GraphQLParser.Tests/Visitors/ASTVisitorTests.cs +++ b/src/GraphQLParser.Tests/Visitors/ASTVisitorTests.cs @@ -4,12 +4,12 @@ namespace GraphQLParser.Tests.Visitors; public class ASTVisitorTests { - private class Context : IASTVisitorContext + private sealed class Context : IASTVisitorContext { public CancellationToken CancellationToken { get; set; } } - private class MySuperNode : ASTNode + private sealed class MySuperNode : ASTNode { public override ASTNodeKind Kind => (ASTNodeKind)12345; } @@ -43,7 +43,7 @@ public void ASTVisitor_Should_Respect_CancellationToken() Should.Throw(() => visitor.VisitAsync(document, context).GetAwaiter().GetResult()); } - private class MyVisitor : ASTVisitor + private sealed class MyVisitor : ASTVisitor { protected override async ValueTask VisitScalarTypeDefinitionAsync(GraphQLScalarTypeDefinition scalarTypeDefinition, Context context) { diff --git a/src/GraphQLParser.Tests/Visitors/GraphQLAstVisitorTests.cs b/src/GraphQLParser.Tests/Visitors/GraphQLAstVisitorTests.cs index 776d1bbb..a07298c6 100644 --- a/src/GraphQLParser.Tests/Visitors/GraphQLAstVisitorTests.cs +++ b/src/GraphQLParser.Tests/Visitors/GraphQLAstVisitorTests.cs @@ -2,10 +2,9 @@ namespace GraphQLParser.Tests.Visitors; -[System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2012:Use ValueTasks correctly", Justification = "CountVisitor is sync")] public class GraphQLAstVisitorTests { - public class CountVisitor : ASTVisitor + internal sealed class CountVisitor : ASTVisitor { protected override async ValueTask VisitBooleanValueAsync(GraphQLBooleanValue booleanValue, CountContext context) { @@ -180,7 +179,7 @@ protected override async ValueTask VisitSchemaDefinitionAsync(GraphQLSchemaDefin } } - public class CountContext : IASTVisitorContext + internal sealed class CountContext : IASTVisitorContext { public List VisitedAliases = new(); public List VisitedArguments = new(); @@ -205,7 +204,7 @@ public class CountContext : IASTVisitorContext private readonly CountVisitor _visitor = new(); - public CountContext Context = new(); + private CountContext Context { get; } = new(); [Theory] [InlineData(IgnoreOptions.None)] diff --git a/src/GraphQLParser.Tests/Visitors/SDLPrinterFromParsedTextTests.cs b/src/GraphQLParser.Tests/Visitors/SDLPrinterFromParsedTextTests.cs index 5e4f98f8..1b8f08fb 100644 --- a/src/GraphQLParser.Tests/Visitors/SDLPrinterFromParsedTextTests.cs +++ b/src/GraphQLParser.Tests/Visitors/SDLPrinterFromParsedTextTests.cs @@ -791,7 +791,7 @@ public async Task SDLPrinter_Should_Throw_On_Unknown_Values(string text) ex.Message.ShouldStartWith("Unknown "); } - private class Context : IASTVisitorContext + private sealed class Context : IASTVisitorContext { public CancellationToken CancellationToken { get; set; } } diff --git a/src/GraphQLParser.Tests/Visitors/SDLPrinterSkipDirectivesTests.cs b/src/GraphQLParser.Tests/Visitors/SDLPrinterSkipDirectivesTests.cs index 08c187e2..18746642 100644 --- a/src/GraphQLParser.Tests/Visitors/SDLPrinterSkipDirectivesTests.cs +++ b/src/GraphQLParser.Tests/Visitors/SDLPrinterSkipDirectivesTests.cs @@ -61,7 +61,7 @@ public async Task Printer_Should_Print_Pretty_If_Directives_Skipped( actual.Parse(); // should be parsed back } - private class MyPrinter : SDLPrinter + private sealed class MyPrinter : SDLPrinter { protected override ValueTask VisitDirectiveAsync(GraphQLDirective directive, DefaultPrintContext context) { diff --git a/src/GraphQLParser.Tests/Visitors/SDLPrinterVerticalIndentationTests.cs b/src/GraphQLParser.Tests/Visitors/SDLPrinterVerticalIndentationTests.cs index 9efd4918..a22b58b5 100644 --- a/src/GraphQLParser.Tests/Visitors/SDLPrinterVerticalIndentationTests.cs +++ b/src/GraphQLParser.Tests/Visitors/SDLPrinterVerticalIndentationTests.cs @@ -193,7 +193,7 @@ public async Task Printer_Should_Print_Pretty_If_Definitions_Skipped( actual.Parse(); // should be parsed back } - private class PrintOnlyStartsWithA : SDLPrinter + private sealed class PrintOnlyStartsWithA : SDLPrinter { protected override ValueTask MakeVerticalIndentationBetweenTopLevelDefinitions(ASTNode node, DefaultPrintContext context) { diff --git a/src/GraphQLParser/AST/Enums/DirectiveLocation.cs b/src/GraphQLParser/AST/Enums/DirectiveLocation.cs index a748423f..ddd32d44 100644 --- a/src/GraphQLParser/AST/Enums/DirectiveLocation.cs +++ b/src/GraphQLParser/AST/Enums/DirectiveLocation.cs @@ -54,7 +54,9 @@ public enum DirectiveLocation /// Location adjacent to an object type definition. [Description("Location adjacent to an object type definition.")] +#pragma warning disable CA1720 // Identifiers should not contain type names Object, +#pragma warning restore CA1720 /// Location adjacent to a field definition. [Description("Location adjacent to a field definition.")] diff --git a/src/GraphQLParser/Exceptions/GraphQLParserException.cs b/src/GraphQLParser/Exceptions/GraphQLParserException.cs index fff0d1d7..720b6c29 100644 --- a/src/GraphQLParser/Exceptions/GraphQLParserException.cs +++ b/src/GraphQLParser/Exceptions/GraphQLParserException.cs @@ -1,3 +1,4 @@ +using System.Globalization; using System.Text; namespace GraphQLParser.Exceptions; @@ -42,9 +43,9 @@ private static string ComposeMessage(string description, ReadOnlySpan sour private static string HighlightSourceAtLocation(ReadOnlySpan source, Location location) { int line = location.Line; - string prevLineNum = (line - 1).ToString(); - string lineNum = line.ToString(); - string nextLineNum = (line + 1).ToString(); + string prevLineNum = (line - 1).ToString(CultureInfo.InvariantCulture); + string lineNum = line.ToString(CultureInfo.InvariantCulture); + string nextLineNum = (line + 1).ToString(CultureInfo.InvariantCulture); int padLen = nextLineNum.Length; string[] lines = source .ToString() @@ -80,7 +81,7 @@ private static string ReplaceWithUnicodeRepresentation(string str) { if (IsReplacementCharacter(code)) { - buffer.Append("\\u").Append(((int)code).ToString("D4")); + buffer.Append("\\u").Append(((int)code).ToString("D4", CultureInfo.InvariantCulture)); } else { diff --git a/src/GraphQLParser/Extensions/ASTNodeExtensions.cs b/src/GraphQLParser/Extensions/ASTNodeExtensions.cs index 4a675a48..f2211bb9 100644 --- a/src/GraphQLParser/Extensions/ASTNodeExtensions.cs +++ b/src/GraphQLParser/Extensions/ASTNodeExtensions.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using GraphQLParser.AST; using GraphQLParser.Visitors; @@ -17,7 +18,9 @@ public static int AllNestedCount(this ASTNode node) { var visitor = new CountVisitor(); var context = new DefaultCountContext(_ => true); - visitor.VisitAsync(node, context).GetAwaiter().GetResult(); // it's safe since method is actually sync + var task = visitor.VisitAsync(node, context); + Debug.Assert(task.IsCompleted); + task.GetAwaiter().GetResult(); // it's safe since method is actually sync that is verified by Debug.Assert return context.Count; } @@ -30,7 +33,9 @@ public static int MaxNestedDepth(this ASTNode node) { var visitor = new MaxDepthVisitor(); var context = new DefaultMaxDepthContext(); - visitor.VisitAsync(node, context).GetAwaiter().GetResult(); // it's safe since method is actually sync + var task = visitor.VisitAsync(node, context); + Debug.Assert(task.IsCompleted); + task.GetAwaiter().GetResult(); // it's safe since method is actually sync that is verified by Debug.Assert return context.MaxDepth; } diff --git a/src/GraphQLParser/TokenKind.cs b/src/GraphQLParser/TokenKind.cs index bb875ed1..3b897077 100644 --- a/src/GraphQLParser/TokenKind.cs +++ b/src/GraphQLParser/TokenKind.cs @@ -82,6 +82,8 @@ public enum TokenKind /// NAME = 15, +#pragma warning disable CA1720 // Identifiers should not contain type names + /// /// An integer number is specified without a decimal point or exponent (ex. 1). /// @@ -110,6 +112,8 @@ public enum TokenKind /// STRING = 18, +#pragma warning restore CA1720 + /// /// GraphQL source documents may contain single‐line comments, starting with the # marker. /// A comment can contain any Unicode code point except LineTerminator so a comment always diff --git a/src/GraphQLParser/Visitors/ASTVisitor.cs b/src/GraphQLParser/Visitors/ASTVisitor.cs index 1a94ffc2..3bf2ad8a 100644 --- a/src/GraphQLParser/Visitors/ASTVisitor.cs +++ b/src/GraphQLParser/Visitors/ASTVisitor.cs @@ -123,7 +123,9 @@ protected virtual async ValueTask VisitSelectionSetAsync(GraphQLSelectionSet sel /// /// Visits node. /// +#pragma warning disable CA1716 // Identifiers should not match keywords (alias) protected virtual async ValueTask VisitAliasAsync(GraphQLAlias alias, TContext context) +#pragma warning restore CA1716 { await VisitAsync(alias.Comments, context).ConfigureAwait(false); await VisitAsync(alias.Name, context).ConfigureAwait(false); diff --git a/src/GraphQLParser/Visitors/SDLPrinter.cs b/src/GraphQLParser/Visitors/SDLPrinter.cs index 75e1da0b..48d23166 100644 --- a/src/GraphQLParser/Visitors/SDLPrinter.cs +++ b/src/GraphQLParser/Visitors/SDLPrinter.cs @@ -1073,14 +1073,14 @@ public DefaultPrintContext(TextWriter writer) public int IndentLevel { get; set; } /// - [Obsolete] + [Obsolete("Use LastVisitedNode instead")] public bool LastDefinitionPrinted { get; set; } /// public bool NewLinePrinted { get; set; } = true; /// - public bool IndentPrinted { get; set; } = false; + public bool IndentPrinted { get; set; } /// public ASTNode? LastVisitedNode { get; set; }