diff --git a/src/Agoda.Analyzers.Test/Agoda.Analyzers.Test.csproj b/src/Agoda.Analyzers.Test/Agoda.Analyzers.Test.csproj index 5a213c3..312b7ad 100644 --- a/src/Agoda.Analyzers.Test/Agoda.Analyzers.Test.csproj +++ b/src/Agoda.Analyzers.Test/Agoda.Analyzers.Test.csproj @@ -26,6 +26,18 @@ prompt 4 + + + + + + + PreserveNewest + + + PreserveNewest + + @@ -35,6 +47,7 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive + @@ -72,6 +85,10 @@ + + + + diff --git a/src/Agoda.Analyzers.Test/AgodaCustom/AG0012UnitTests.cs b/src/Agoda.Analyzers.Test/AgodaCustom/AG0012UnitTests.cs index cb71a00..a658d5b 100644 --- a/src/Agoda.Analyzers.Test/AgodaCustom/AG0012UnitTests.cs +++ b/src/Agoda.Analyzers.Test/AgodaCustom/AG0012UnitTests.cs @@ -1,8 +1,12 @@ -using Agoda.Analyzers.AgodaCustom; +using System.Reflection; +using Agoda.Analyzers.AgodaCustom; using Agoda.Analyzers.Test.Helpers; using Microsoft.CodeAnalysis.Diagnostics; using NUnit.Framework; using System.Threading.Tasks; +using System.Xml; +using System.Xml.Linq; +using FluentAssertions; namespace Agoda.Analyzers.Test.AgodaCustom; @@ -87,12 +91,66 @@ public void This_Is_Valid() } }" }; - await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults); } + + [Test] + public async Task AG0012_WithXUnitAssertion_ShouldNotShowWarning() + { + var code = new CodeDescriptor + { + References = new[] { typeof(Xunit.FactAttribute).Assembly, typeof(Xunit.Assert).Assembly }, + Code = @" + using Xunit; + namespace Tests + { + public class TestClass + { + [Fact] + public void This_Is_Valid() + { + int[] arrayToAssert = { 1, 2, 3 }; + Assert.NotNull(arrayToAssert); + } + } + }" + }; + await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults); + } + [Test] + public async Task AG0012_WithFluentAssertions_ShouldNotShowWarning() + { + var code = new CodeDescriptor + { + References = new[] { typeof(NUnitAttribute).Assembly, + typeof(FluentAssertions.TypeExtensions).Assembly, + typeof(XDocument).Assembly, + typeof(XmlNode).Assembly, + Assembly.LoadFrom("System.Xml.XDocument.dll"), + Assembly.LoadFrom("System.Xml.ReaderWriter.dll"), + }, + Code = @" + using NUnit.Framework; + using FluentAssertions; + + namespace Tests + { + internal class TestClass + { + [Test] + internal void This_Is_Valid() + { + int arrayToAssert = 1; + arrayToAssert.Should().Be(1); + } + } + }" + }; + await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults); + } [Test] public async Task AG0012_WithShouldlyAssertion_ShouldNotShowWarning() { diff --git a/src/Agoda.Analyzers.Test/AgodaCustom/AG0039UnitTests.cs b/src/Agoda.Analyzers.Test/AgodaCustom/AG0039UnitTests.cs index de444f9..4812139 100644 --- a/src/Agoda.Analyzers.Test/AgodaCustom/AG0039UnitTests.cs +++ b/src/Agoda.Analyzers.Test/AgodaCustom/AG0039UnitTests.cs @@ -8,107 +8,51 @@ namespace Agoda.Analyzers.Test.AgodaCustom; class AG0039UnitTests : DiagnosticVerifier { - protected override DiagnosticAnalyzer DiagnosticAnalyzer => new AG0039UndocumentedMemberAnalyzer(); - - protected override string DiagnosticId => AG0039UndocumentedMemberAnalyzer.DIAGNOSTIC_ID; + protected override DiagnosticAnalyzer DiagnosticAnalyzer => new AG0039MethodLineLengthAnalyzer(); + protected override string DiagnosticId => AG0039MethodLineLengthAnalyzer.DIAGNOSTIC_ID; + [Test] - [TestCase(@" - /// - /// - /// - public class NotDoc - { - /// - /// - /// - public string str1 { get; } - ///// - ///// - ///// - //public const int int2 = 1; - ///// - ///// - ///// - //public void DoesNothing() { } - ///// - ///// - ///// - //public event SampleEventHandler SampleEvent; - ///// - ///// - ///// - ///// - //public delegate void SampleEventHandler(object sender); - } -")] - [TestCase(@" - /// - public class NotDoc + public async Task AG0039_At40LinesOfCode_ShowWarning() + { + var code = @" + internal class InternalClass { - /// - public string str1 { get; } - /// - public const int int2 = 1; - /// - public void DoesNothing() { } - /// - public event SampleEventHandler SampleEvent; - /// - public delegate void SampleEventHandler(object sender); + private void MyMethod() + { +var a = 1; +"; + for (int i = 0; i <= 121; i++) + { + code += "a += 1;\n"; + } + code += @" + } } -")] - public async Task AG0039_WithDoc_ShowNoWarning(string code) - { - await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults); +"; + await VerifyDiagnosticsAsync(code, new[]{ + new DiagnosticLocation(4, 34, "MyMethod","126", "40"), + }); } [Test] - [TestCase(@" - public class NotDoc - { - public string str1 {get;} - public const int int2 = 1; - public void DoesNothing() {} - public event SampleEventHandler SampleEvent; - public delegate void SampleEventHandler(object sender); - internal string internalString; - internal string internalStringProp {get;} - protected string protectedString; - private int _privateInt; - } + public async Task AG0039_At40LinesOfWhiteSpace_DoNotShowWarning() + { + var code = @" internal class InternalClass { - public string str1 {get;} - public const int int2 = 1; - public void DoesNothing() {} - public event SampleEventHandler SampleEvent; - public delegate void SampleEventHandler(object sender); - internal string internalString; - internal string internalStringProp {get;} - protected string protectedString; - private int _privateInt; - - private class MyClass + private void MyMethod() { +var a = 1; +"; + for (int i = 0; i <= 121; i++) + { + code += "\n"; + } + code += @" } } -")] - public async Task AG0039_WithoutDoc_ShowWarning(string code) - { - - await VerifyDiagnosticsAsync(code, new[]{ - new DiagnosticLocation(2, 18), - new DiagnosticLocation(4, 20), - new DiagnosticLocation(5, 23), - new DiagnosticLocation(6, 33), - new DiagnosticLocation(7, 53), - new DiagnosticLocation(8, 42), - new DiagnosticLocation(16, 35), - new DiagnosticLocation(17, 38), - new DiagnosticLocation(18, 33), - new DiagnosticLocation(19, 53), - new DiagnosticLocation(20, 42) - }); +"; + await VerifyDiagnosticsAsync(code,EmptyDiagnosticResults); } } diff --git a/src/Agoda.Analyzers.Test/Helpers/DiagnosticLocation.cs b/src/Agoda.Analyzers.Test/Helpers/DiagnosticLocation.cs index 8c35d6e..a6239bc 100644 --- a/src/Agoda.Analyzers.Test/Helpers/DiagnosticLocation.cs +++ b/src/Agoda.Analyzers.Test/Helpers/DiagnosticLocation.cs @@ -6,7 +6,13 @@ public class DiagnosticLocation { public int Line { get; } public int Col { get; } - + public string[] Args { get; set; } + public DiagnosticLocation(int line, int col, params string[] args) + { + Line = line; + Col = col; + Args = args; + } public DiagnosticLocation(int line, int col) { if (line <= 0 || col <= 0) @@ -15,5 +21,6 @@ public DiagnosticLocation(int line, int col) } Line = line; Col = col; + Args = Array.Empty(); } } \ No newline at end of file diff --git a/src/Agoda.Analyzers.Test/Helpers/DiagnosticVerifier.cs b/src/Agoda.Analyzers.Test/Helpers/DiagnosticVerifier.cs index 2e7c479..091c929 100644 --- a/src/Agoda.Analyzers.Test/Helpers/DiagnosticVerifier.cs +++ b/src/Agoda.Analyzers.Test/Helpers/DiagnosticVerifier.cs @@ -103,7 +103,7 @@ protected async Task VerifyDiagnosticsAsync(CodeDescriptor descriptor, Diagnosti protected async Task VerifyDiagnosticsAsync(CodeDescriptor descriptor, DiagnosticLocation[] expectedLocations) { var baseResult = CSharpDiagnostic(DiagnosticId); - var expected = expectedLocations.Select(l => baseResult.WithLocation(l.Line, l.Col)).ToArray(); + var expected = expectedLocations.Select(l => baseResult.WithLocation(l.Line, l.Col).WithArguments(l.Args)).ToArray(); var doc = CreateProject(new[] {descriptor.Code}) diff --git a/src/Agoda.Analyzers.Test/System.Xml.ReaderWriter.dll b/src/Agoda.Analyzers.Test/System.Xml.ReaderWriter.dll new file mode 100644 index 0000000..08e4ce8 Binary files /dev/null and b/src/Agoda.Analyzers.Test/System.Xml.ReaderWriter.dll differ diff --git a/src/Agoda.Analyzers.Test/System.Xml.XDocument.dll b/src/Agoda.Analyzers.Test/System.Xml.XDocument.dll new file mode 100644 index 0000000..3e41a45 Binary files /dev/null and b/src/Agoda.Analyzers.Test/System.Xml.XDocument.dll differ diff --git a/src/Agoda.Analyzers/Agoda.Analyzers.csproj b/src/Agoda.Analyzers/Agoda.Analyzers.csproj index c985186..91f093d 100644 --- a/src/Agoda.Analyzers/Agoda.Analyzers.csproj +++ b/src/Agoda.Analyzers/Agoda.Analyzers.csproj @@ -92,9 +92,6 @@ - - PreserveNewest - Always diff --git a/src/Agoda.Analyzers/AgodaCustom/AG0012TestMethodMustContainAtLeastOneAssertion.cs b/src/Agoda.Analyzers/AgodaCustom/AG0012TestMethodMustContainAtLeastOneAssertion.cs index 5ac221b..90914f5 100644 --- a/src/Agoda.Analyzers/AgodaCustom/AG0012TestMethodMustContainAtLeastOneAssertion.cs +++ b/src/Agoda.Analyzers/AgodaCustom/AG0012TestMethodMustContainAtLeastOneAssertion.cs @@ -5,6 +5,7 @@ using Microsoft.CodeAnalysis.Diagnostics; using System.Collections.Immutable; using System.Linq; +using System.Net; namespace Agoda.Analyzers.AgodaCustom { @@ -43,6 +44,7 @@ private static readonly LocalizableString Description { new AssertLibraryInfo("NUnit.Framework", "nunit.framework.dll", "Assert"), new AssertLibraryInfo("Shouldly", "Shouldly.dll", "Should", "Shouldly.Should"), + //FluentAssertions }; public override void Initialize(AnalysisContext context) @@ -84,20 +86,27 @@ private static bool HasInvokedAssertStaticMethod(MethodDeclarationSyntax methodD private static bool HasInvokedAssertExtensionMethod(MethodDeclarationSyntax methodDeclaration, SyntaxNodeAnalysisContext context) { - return methodDeclaration.Body.Statements + var i = methodDeclaration.Body.Statements .SelectMany(s => s.DescendantNodesAndSelf()) .OfType() .Select(ies => ies.Expression) - .OfType() - .Select(mae => mae.Name) - .OfType() - .Select(ins => context.SemanticModel.GetSymbolInfo(ins).Symbol) - .Any(symbol => AssertionLibraryList + .OfType(); + var z = i.Select(mae => mae.Name) + .OfType(); + foreach (var identifierNameSyntax in z) + { + var t = context.SemanticModel.GetSymbolInfo(identifierNameSyntax); + var u = context.SemanticModel.GetDiagnostics(); + var r = context.SemanticModel.GetDeclaredSymbol(identifierNameSyntax); + } + var y = z.Select(ins => context.SemanticModel.GetSymbolInfo(ins).Symbol) + .Any(symbol => AssertionLibraryList .Where(lib => lib.HasExtenstionMethods).Any(lib => symbol.ContainingNamespace.ToDisplayString() == lib.Namespace && symbol.ContainingModule.ToDisplayString() == lib.Module && symbol.ContainingType.ToDisplayString().StartsWith(lib.Type) && symbol.Name.StartsWith(lib.Name))); + return y; } private class AssertLibraryInfo diff --git a/src/Agoda.Analyzers/AgodaCustom/AG0039MethodLineLengthAnalyzer.cs b/src/Agoda.Analyzers/AgodaCustom/AG0039MethodLineLengthAnalyzer.cs new file mode 100644 index 0000000..a2cfd87 --- /dev/null +++ b/src/Agoda.Analyzers/AgodaCustom/AG0039MethodLineLengthAnalyzer.cs @@ -0,0 +1,81 @@ +using System.Collections.Immutable; +using System.Resources; +using Agoda.Analyzers.Helpers; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Text; +using System.Collections.Generic; +using System.Data; +using System.Linq; + +namespace Agoda.Analyzers.AgodaCustom +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class AG0039MethodLineLengthAnalyzer : DiagnosticAnalyzer + { + private const int MAX_LINE_DEFAULT = 40; // Adjust the maximum allowed lines as needed. + public const string DIAGNOSTIC_ID = "AG0039"; + private int MaxLines = MAX_LINE_DEFAULT; + private static readonly LocalizableString Title = new LocalizableResourceString( + nameof(CustomRulesResources.AG0039Title), + CustomRulesResources.ResourceManager, + typeof(CustomRulesResources)); + + private static readonly LocalizableString MessageFormat = new LocalizableResourceString( + nameof(CustomRulesResources.AG0039Title), + CustomRulesResources.ResourceManager, + typeof(CustomRulesResources)); + + private static readonly LocalizableString Description + = DescriptionContentLoader.GetAnalyzerDescription(nameof(AG0039MethodLineLengthAnalyzer)); + + private const string Category = "Documentation"; + private static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor( + DIAGNOSTIC_ID, + Title, + MessageFormat, + Category, + DiagnosticSeverity.Hidden, // THis rule should be opt in and not on by default + isEnabledByDefault: true, + helpLinkUri: "https://github.com/agoda-com/AgodaAnalyzers/blob/master/src/Agoda.Analyzers/RuleContent/AG0039MethodLineLengthAnalyzer.html"); + + public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(Descriptor); } } + + public override void Initialize(AnalysisContext context) + { + context.RegisterCompilationStartAction(compilationContext => + { + var optionsProvider = compilationContext.Options.AnalyzerConfigOptionsProvider; + var options = optionsProvider.GetOptions(compilationContext.Compilation.SyntaxTrees.First()); + + if (options.TryGetValue("dotnet_diagnostic.AG0039.method_line_length_limit", out var configValue)) + { + if (int.TryParse(configValue, out var i)) + { + MaxLines = i; + } + } + + compilationContext.RegisterSyntaxNodeAction(AnalyzeMethod, SyntaxKind.MethodDeclaration); + }); + } + + private void AnalyzeMethod(SyntaxNodeAnalysisContext context) + { + var methodNode = (MethodDeclarationSyntax)context.Node; + var lineCount = GetNonEmptyLineCount(methodNode.GetText().Lines); + + if (lineCount <= MaxLines) return; + + var diagnostic = Diagnostic.Create(Descriptor, methodNode.Identifier.GetLocation(), methodNode.Identifier.Text, lineCount, MaxLines); + context.ReportDiagnostic(diagnostic); + } + + private static int GetNonEmptyLineCount(IEnumerable lines) + { + return lines.Count(line => !string.IsNullOrWhiteSpace(line.ToString())); + } + } +} \ No newline at end of file diff --git a/src/Agoda.Analyzers/AgodaCustom/AG0039UndocumentedMemberAnalyzer.cs b/src/Agoda.Analyzers/AgodaCustom/AG0039UndocumentedMemberAnalyzer.cs deleted file mode 100644 index b6529ca..0000000 --- a/src/Agoda.Analyzers/AgodaCustom/AG0039UndocumentedMemberAnalyzer.cs +++ /dev/null @@ -1,67 +0,0 @@ -using System.Collections.Immutable; -using Agoda.Analyzers.Helpers; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.Diagnostics; - -namespace Agoda.Analyzers.AgodaCustom -{ - [DiagnosticAnalyzer(LanguageNames.CSharp)] - public class AG0039UndocumentedMemberAnalyzer : DiagnosticAnalyzer - { - public const string DIAGNOSTIC_ID = "AG0039"; - - private static readonly LocalizableString Title = new LocalizableResourceString( - nameof(CustomRulesResources.AG0039Title), - CustomRulesResources.ResourceManager, - typeof(CustomRulesResources)); - - private static readonly LocalizableString MessageFormat = new LocalizableResourceString( - nameof(CustomRulesResources.AG0039Title), - CustomRulesResources.ResourceManager, - typeof(CustomRulesResources)); - - private static readonly LocalizableString Description - = DescriptionContentLoader.GetAnalyzerDescription(nameof(AG0039UndocumentedMemberAnalyzer)); - - private const string Category = "Documentation"; - private static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor( - DIAGNOSTIC_ID, - Title, - MessageFormat, - Category, - DiagnosticSeverity.Warning, - isEnabledByDefault: true, - description: "Public surfaces should be documented, please add XML documentation."); - - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Descriptor); - - public override void Initialize(AnalysisContext context) - { - context.RegisterSymbolAction(AnalyzeSymbol, SymbolKind.NamedType, SymbolKind.Method, SymbolKind.Property, SymbolKind.Field, SymbolKind.Event); - } - - private static void AnalyzeSymbol(SymbolAnalysisContext context) - { - if (context.Symbol.DeclaredAccessibility != Accessibility.Public) - return; - - if (!string.IsNullOrWhiteSpace(context.Symbol.GetDocumentationCommentXml())) - return; - - // ignore getters and setter methods for documentation - if (context.Symbol.Kind == SymbolKind.Method) - { - var methodSymbol = (IMethodSymbol)context.Symbol; - if (methodSymbol.MethodKind == MethodKind.PropertyGet || - methodSymbol.MethodKind == MethodKind.PropertySet) - return; - } - - var location = context.Symbol.Locations[0]; - var name = context.Symbol.Name; - - var diagnostic = Diagnostic.Create(Descriptor, location, context.Symbol.Kind.ToString(), name); - context.ReportDiagnostic(diagnostic); - } - } -} \ No newline at end of file diff --git a/src/Agoda.Analyzers/AgodaCustom/CustomRulesResources.Designer.cs b/src/Agoda.Analyzers/AgodaCustom/CustomRulesResources.Designer.cs index f1db291..7c4b06f 100644 --- a/src/Agoda.Analyzers/AgodaCustom/CustomRulesResources.Designer.cs +++ b/src/Agoda.Analyzers/AgodaCustom/CustomRulesResources.Designer.cs @@ -441,7 +441,7 @@ public static string AG0038Title { } /// - /// Looks up a localized string similar to Libraries with Public members must have documentation. + /// Looks up a localized string similar to Method {0} has {1} lines of code and is potentially not readable, consider refactoring for better readability. {2} non-whitespace lines is what we recommend as a maximum, but its up to individual context.. /// public static string AG0039Title { get { diff --git a/src/Agoda.Analyzers/AgodaCustom/CustomRulesResources.resx b/src/Agoda.Analyzers/AgodaCustom/CustomRulesResources.resx index 924e99a..c44c768 100644 --- a/src/Agoda.Analyzers/AgodaCustom/CustomRulesResources.resx +++ b/src/Agoda.Analyzers/AgodaCustom/CustomRulesResources.resx @@ -249,6 +249,6 @@ One exception is logging, where it can be useful to see the exact DC / cluster / Do not use #region directives - Libraries with Public members must have documentation + Method {0} has {1} lines of code and is potentially not readable, consider refactoring for better readability. {2} non-whitespace lines is what we recommend as a maximum, but its up to individual context. \ No newline at end of file diff --git a/src/Agoda.Analyzers/RuleContent/AG0039MethodLineLengthAnalyzer.html b/src/Agoda.Analyzers/RuleContent/AG0039MethodLineLengthAnalyzer.html new file mode 100644 index 0000000..9ff1cae --- /dev/null +++ b/src/Agoda.Analyzers/RuleContent/AG0039MethodLineLengthAnalyzer.html @@ -0,0 +1,71 @@ +

+ Long methods should be considered for refactoring +

+
    +
  • + This rule is to help people identify potentially long methods, so they can consider refactoring them. +
  • +
  • + The rule does not count lines of whitespace in the recommended maximum is 40 lines, this varies per context though and should not be looked at as a hard limit. +
  • +
  • + Try to break up long methods into smaller private methods that describe what's going on. +
  • +
  • + However, don't chain methods together to avoid long methods when refactoring. +
  • +

    Noncompliant Code Example

    +
    +class MyClass()
    +{
    +    public void MethodThatDoesALotOfStuff()
    +    {
    +        return DoThisFirst();
    +    }
    +
    +    private int DoThisFirst()
    +    {
    +    // do some stuff...
    +        return DoThisSecond()
    +    }
    +    private int DoThisSecond()
    +    {
    +    // do some stuff...
    +        return DoThisThird();
    +    }
    +    private int DoThisThird()
    +    {
    +    // do some stuff...
    +    }
    +
    +}
    +    
    +

    Compliant Code Example

    +
    +class MyClass()
    +{
    +    public void MethodThatDoesALotOfStuff()
    +    {
    +        DoThisFirst();
    +        DoThisSecond();
    +        return DoThisThird();
    +    }
    +
    +    private void DoThisFirst()
    +    {
    +    // do some stuff...
    +    }
    +    private void DoThisSecond()
    +    {
    +    // do some stuff...
    +    }
    +    private int DoThisThird()
    +    {
    +    // do some stuff...
    +    }
    +}
    +    
    +
  • + If you are having problems with breaking methods down, try Martin Fowler's Book called refactoring https://www.amazon.com/Refactoring-Improving-Existing-Addison-Wesley-Signature/dp/0134757599/ +
  • +
\ No newline at end of file diff --git a/src/Agoda.Analyzers/RuleContent/AG0039UndocumentedMemberAnalyzer.html b/src/Agoda.Analyzers/RuleContent/AG0039UndocumentedMemberAnalyzer.html deleted file mode 100644 index 2f2de7b..0000000 --- a/src/Agoda.Analyzers/RuleContent/AG0039UndocumentedMemberAnalyzer.html +++ /dev/null @@ -1,19 +0,0 @@ -

- Public members should be documented or made internal -

-
    -
  • - This rule is intended for libraries, it helps other people understand how to use teh code you publish. -
  • -
  • - We follow Docs as Code (https://www.writethedocs.org/guide/docs-as-code/), so we want to document the code in the code, not in a separate document. - This helps ensure that we can the documentation updated, as we can use the same tools we use for code to ensure we update the documentation. - For example, this static code analysis rule -
  • -
  • - Recommend using it with tools like DocFx to complete the documentation loop from code to make it easier for people to consume. -
  • -
  • - Or make them internal, sometimes you don''t want to documentation them because you don't intend people to use them, if this is the case, make them internal. -
  • -
\ No newline at end of file