From 7c7f24eebc6bf96d05751ea02b55b9b8a667fbdd Mon Sep 17 00:00:00 2001 From: Marek Linka Date: Wed, 6 Feb 2019 16:20:22 +0100 Subject: [PATCH 1/3] Add analyzer for detecting BestFriend usages on public declaration --- .../Code/BestFriendOnPublicDeclarationTest.cs | 62 +++++++++++ .../BestFriendOnPublicDeclaration.cs | 97 +++++++++++++++++ .../BestFriendOnPublicDeclarationsAnalyzer.cs | 102 ++++++++++++++++++ 3 files changed, 261 insertions(+) create mode 100644 test/Microsoft.ML.CodeAnalyzer.Tests/Code/BestFriendOnPublicDeclarationTest.cs create mode 100644 test/Microsoft.ML.CodeAnalyzer.Tests/Resources/BestFriendOnPublicDeclaration.cs create mode 100644 tools-local/Microsoft.ML.InternalCodeAnalyzer/BestFriendOnPublicDeclarationsAnalyzer.cs diff --git a/test/Microsoft.ML.CodeAnalyzer.Tests/Code/BestFriendOnPublicDeclarationTest.cs b/test/Microsoft.ML.CodeAnalyzer.Tests/Code/BestFriendOnPublicDeclarationTest.cs new file mode 100644 index 0000000000..c5e1453aaa --- /dev/null +++ b/test/Microsoft.ML.CodeAnalyzer.Tests/Code/BestFriendOnPublicDeclarationTest.cs @@ -0,0 +1,62 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Reflection; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.ML.CodeAnalyzer.Tests.Helpers; +using Xunit; + +namespace Microsoft.ML.InternalCodeAnalyzer.Tests +{ + public sealed class BestFriendOnPublicDeclarationTest : DiagnosticVerifier + { + private readonly Lazy SourceAttribute = TestUtils.LazySource("BestFriendAttribute.cs"); + private readonly Lazy SourceDeclaration = TestUtils.LazySource("BestFriendOnPublicDeclaration.cs"); + + [Fact] + public void BestFriendOnPublicDeclaration() + { + Solution solution = null; + var projA = CreateProject("ProjectA", ref solution, SourceDeclaration.Value, SourceAttribute.Value); + + var analyzer = new BestFriendOnPublicDeclarationsAnalyzer(); + + var refs = new List { + RefFromType(), RefFromType(), + MetadataReference.CreateFromFile(Assembly.Load("netstandard, Version=2.0.0.0").Location), + MetadataReference.CreateFromFile(Assembly.Load("System.Runtime, Version=0.0.0.0").Location) + }; + + var comp = projA.GetCompilationAsync().Result.WithReferences(refs.ToArray()); + var compilationWithAnalyzers = comp.WithAnalyzers(ImmutableArray.Create((DiagnosticAnalyzer)analyzer)); + var allDiags = compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync().Result; + + var projectTrees = new HashSet(projA.Documents.Select(r => r.GetSyntaxTreeAsync().Result)); + var diags = allDiags + .Where(d => d.Location == Location.None || d.Location.IsInMetadata || projectTrees.Contains(d.Location.SourceTree)) + .OrderBy(d => d.Location.SourceSpan.Start).ToArray(); + + var diag = analyzer.SupportedDiagnostics[0]; + var expected = new DiagnosticResult[] { + diag.CreateDiagnosticResult(8, 6, "PublicClass"), + diag.CreateDiagnosticResult(11, 10, "PublicField"), + diag.CreateDiagnosticResult(14, 10, "PublicProperty"), + diag.CreateDiagnosticResult(20, 10, "PublicMethod"), + diag.CreateDiagnosticResult(26, 10, "PublicDelegate"), + diag.CreateDiagnosticResult(29, 10, "PublicClass"), + diag.CreateDiagnosticResult(35, 6, "PublicStruct"), + diag.CreateDiagnosticResult(40, 6, "PublicEnum"), + diag.CreateDiagnosticResult(47, 6, "PublicInterface") + }; + + VerifyDiagnosticResults(diags, analyzer, expected); + } + } +} + diff --git a/test/Microsoft.ML.CodeAnalyzer.Tests/Resources/BestFriendOnPublicDeclaration.cs b/test/Microsoft.ML.CodeAnalyzer.Tests/Resources/BestFriendOnPublicDeclaration.cs new file mode 100644 index 0000000000..882c499ec3 --- /dev/null +++ b/test/Microsoft.ML.CodeAnalyzer.Tests/Resources/BestFriendOnPublicDeclaration.cs @@ -0,0 +1,97 @@ +using System; +using Microsoft.ML; + +namespace TestNamespace +{ + // all of the best friend declaration should fail the diagnostic + + [BestFriend] + public class PublicClass + { + [BestFriend] + public int PublicField; + + [BestFriend] + public string PublicProperty + { + get { return string.Empty; } + } + + [BestFriend] + public bool PublicMethod() + { + return true; + } + + [BestFriend] + public delegate string PublicDelegate(); + + [BestFriend] + public PublicClass() + { + } + } + + [BestFriend] + public struct PublicStruct + { + } + + [BestFriend] + public enum PublicEnum + { + EnumValue1, + EnumValue2 + } + + [BestFriend] + public interface PublicInterface + { + } + + // these should work + + [BestFriend] + internal class InternalClass + { + [BestFriend] + internal int InternalField; + + [BestFriend] + internal string InternalProperty + { + get { return string.Empty; } + } + + [BestFriend] + internal bool InternalMethod() + { + return true; + } + + [BestFriend] + internal delegate string InternalDelegate(); + + [BestFriend] + internal InternalClass() + { + } + } + + [BestFriend] + internal struct InternalStruct + { + } + + [BestFriend] + internal enum InternalEnum + { + EnumValue1, + EnumValue2 + } + + [BestFriend] + internal interface InternalInterface + { + } +} diff --git a/tools-local/Microsoft.ML.InternalCodeAnalyzer/BestFriendOnPublicDeclarationsAnalyzer.cs b/tools-local/Microsoft.ML.InternalCodeAnalyzer/BestFriendOnPublicDeclarationsAnalyzer.cs new file mode 100644 index 0000000000..20efec4b83 --- /dev/null +++ b/tools-local/Microsoft.ML.InternalCodeAnalyzer/BestFriendOnPublicDeclarationsAnalyzer.cs @@ -0,0 +1,102 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Collections.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.ML.InternalCodeAnalyzer +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class BestFriendOnPublicDeclarationsAnalyzer : DiagnosticAnalyzer + { + private const string Category = "Access"; + internal const string DiagnosticId = "MSML_BestFriendOnPublicDeclaration"; + + private const string Title = "Public declarations should not have " + AttributeName + " attribute."; + private const string Format = "The " + AttributeName + " should not be applied to publicly visible members."; + + private const string Description = + "The " + AttributeName + " attribute is only valid on internal identifiers."; + + private static DiagnosticDescriptor Rule = + new DiagnosticDescriptor(DiagnosticId, Title, Format, Category, + DiagnosticSeverity.Warning, isEnabledByDefault: true, description: Description); + + private const string AttributeName = "Microsoft.ML.BestFriendAttribute"; + + public override ImmutableArray SupportedDiagnostics => + ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) + { + context.RegisterSemanticModelAction(Analyze); + } + + private void AnalyzeCore(SemanticModelAnalysisContext context, string attributeName) + { + var model = context.SemanticModel; + var comp = model.Compilation; + + // Get the symbols of the key types we are analyzing. If we can't find it + // there is no point in going further. + var bestFriendAttributeType = comp.GetTypeByMetadataName(attributeName); + if (bestFriendAttributeType == null) + return; + + foreach (var node in model.SyntaxTree.GetRoot().DescendantNodes(n => !n.IsKind(SyntaxKind.UsingDirective))) + { + switch (node.Kind()) + { + case SyntaxKind.ClassDeclaration: + case SyntaxKind.StructDeclaration: + case SyntaxKind.InterfaceDeclaration: + case SyntaxKind.EnumDeclaration: + case SyntaxKind.MethodDeclaration: + case SyntaxKind.ConstructorDeclaration: + case SyntaxKind.PropertyDeclaration: + case SyntaxKind.DelegateDeclaration: + var declaredSymbol = model.GetDeclaredSymbol(node); + if (declaredSymbol == null) + continue; + + VerifySymbol(context, declaredSymbol, bestFriendAttributeType); + break; + case SyntaxKind.FieldDeclaration: // field declaration is a little more complicated as it needs to be decomposed + foreach (var variable in ((FieldDeclarationSyntax)node).Declaration.Variables) + { + var fieldSymbol = model.GetDeclaredSymbol(variable); + VerifySymbol(context, fieldSymbol, bestFriendAttributeType); + } + break; + default: + continue; + } + } + } + + private static void VerifySymbol(SemanticModelAnalysisContext context, ISymbol symbol, + INamedTypeSymbol bestFriendAttributeType) + { + if (symbol.DeclaredAccessibility != Accessibility.Public) + return; + + var attribute = symbol.GetAttributes().FirstOrDefault(a => a.AttributeClass == bestFriendAttributeType); + if (attribute != null) + { + var diagnostic = Diagnostic.Create(Rule, attribute.ApplicationSyntaxReference.GetSyntax().GetLocation(), symbol.Name); + context.ReportDiagnostic(diagnostic); + } + } + + private void Analyze(SemanticModelAnalysisContext context) + { + AnalyzeCore(context, "Microsoft.ML.BestFriendAttribute"); + AnalyzeCore(context, "Microsoft.ML.Internal.CpuMath.Core.BestFriendAttribute"); + } + } +} \ No newline at end of file From 525ce3815e591f133dd1c7689e795c0727f89cde Mon Sep 17 00:00:00 2001 From: Marek Linka Date: Thu, 7 Feb 2019 07:49:41 +0100 Subject: [PATCH 2/3] Rewrite analyzer as Symbol analyzer --- .../BestFriendOnPublicDeclarationsAnalyzer.cs | 74 ++++++------------- 1 file changed, 21 insertions(+), 53 deletions(-) diff --git a/tools-local/Microsoft.ML.InternalCodeAnalyzer/BestFriendOnPublicDeclarationsAnalyzer.cs b/tools-local/Microsoft.ML.InternalCodeAnalyzer/BestFriendOnPublicDeclarationsAnalyzer.cs index 20efec4b83..658e957c16 100644 --- a/tools-local/Microsoft.ML.InternalCodeAnalyzer/BestFriendOnPublicDeclarationsAnalyzer.cs +++ b/tools-local/Microsoft.ML.InternalCodeAnalyzer/BestFriendOnPublicDeclarationsAnalyzer.cs @@ -2,11 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; namespace Microsoft.ML.InternalCodeAnalyzer @@ -21,7 +20,7 @@ public sealed class BestFriendOnPublicDeclarationsAnalyzer : DiagnosticAnalyzer private const string Format = "The " + AttributeName + " should not be applied to publicly visible members."; private const string Description = - "The " + AttributeName + " attribute is only valid on internal identifiers."; + "The " + AttributeName + " attribute is not valid on public identifiers."; private static DiagnosticDescriptor Rule = new DiagnosticDescriptor(DiagnosticId, Title, Format, Category, @@ -34,69 +33,38 @@ public sealed class BestFriendOnPublicDeclarationsAnalyzer : DiagnosticAnalyzer public override void Initialize(AnalysisContext context) { - context.RegisterSemanticModelAction(Analyze); + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + + context.RegisterCompilationStartAction(CompilationStart); } - private void AnalyzeCore(SemanticModelAnalysisContext context, string attributeName) + private void CompilationStart(CompilationStartAnalysisContext context) { - var model = context.SemanticModel; - var comp = model.Compilation; - - // Get the symbols of the key types we are analyzing. If we can't find it - // there is no point in going further. - var bestFriendAttributeType = comp.GetTypeByMetadataName(attributeName); - if (bestFriendAttributeType == null) - return; + var list = new List { AttributeName, "Microsoft.ML.Internal.CpuMath.Core.BestFriendAttribute" }; - foreach (var node in model.SyntaxTree.GetRoot().DescendantNodes(n => !n.IsKind(SyntaxKind.UsingDirective))) + foreach (var attributeName in list) { - switch (node.Kind()) - { - case SyntaxKind.ClassDeclaration: - case SyntaxKind.StructDeclaration: - case SyntaxKind.InterfaceDeclaration: - case SyntaxKind.EnumDeclaration: - case SyntaxKind.MethodDeclaration: - case SyntaxKind.ConstructorDeclaration: - case SyntaxKind.PropertyDeclaration: - case SyntaxKind.DelegateDeclaration: - var declaredSymbol = model.GetDeclaredSymbol(node); - if (declaredSymbol == null) - continue; + var attribute = context.Compilation.GetTypeByMetadataName(attributeName); - VerifySymbol(context, declaredSymbol, bestFriendAttributeType); - break; - case SyntaxKind.FieldDeclaration: // field declaration is a little more complicated as it needs to be decomposed - foreach (var variable in ((FieldDeclarationSyntax)node).Declaration.Variables) - { - var fieldSymbol = model.GetDeclaredSymbol(variable); - VerifySymbol(context, fieldSymbol, bestFriendAttributeType); - } - break; - default: - continue; - } + if (attribute == null) + continue; + + context.RegisterSymbolAction(c => AnalyzeCore(c, attribute), SymbolKind.NamedType, SymbolKind.Method, SymbolKind.Field, SymbolKind.Property); } } - private static void VerifySymbol(SemanticModelAnalysisContext context, ISymbol symbol, - INamedTypeSymbol bestFriendAttributeType) + private void AnalyzeCore(SymbolAnalysisContext context, INamedTypeSymbol attributeType) { - if (symbol.DeclaredAccessibility != Accessibility.Public) + if (context.Symbol.DeclaredAccessibility != Accessibility.Public) return; - var attribute = symbol.GetAttributes().FirstOrDefault(a => a.AttributeClass == bestFriendAttributeType); - if (attribute != null) - { - var diagnostic = Diagnostic.Create(Rule, attribute.ApplicationSyntaxReference.GetSyntax().GetLocation(), symbol.Name); - context.ReportDiagnostic(diagnostic); - } - } + var attribute = context.Symbol.GetAttributes().FirstOrDefault(a => a.AttributeClass == attributeType); + if (attribute == null) + return; - private void Analyze(SemanticModelAnalysisContext context) - { - AnalyzeCore(context, "Microsoft.ML.BestFriendAttribute"); - AnalyzeCore(context, "Microsoft.ML.Internal.CpuMath.Core.BestFriendAttribute"); + var diagnostic = Diagnostic.Create(Rule, attribute.ApplicationSyntaxReference.GetSyntax().GetLocation(), context.Symbol.Name); + context.ReportDiagnostic(diagnostic); } } } \ No newline at end of file From 5e0d7c598d2a96762fae9eca07d04c089262bbc8 Mon Sep 17 00:00:00 2001 From: Marek Linka Date: Fri, 8 Feb 2019 17:46:26 +0100 Subject: [PATCH 3/3] Add test to prove IntClass.PubMember reports a warning --- .../Code/BestFriendOnPublicDeclarationTest.cs | 3 ++- .../Resources/BestFriendOnPublicDeclaration.cs | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/test/Microsoft.ML.CodeAnalyzer.Tests/Code/BestFriendOnPublicDeclarationTest.cs b/test/Microsoft.ML.CodeAnalyzer.Tests/Code/BestFriendOnPublicDeclarationTest.cs index c5e1453aaa..ddd8fb5d72 100644 --- a/test/Microsoft.ML.CodeAnalyzer.Tests/Code/BestFriendOnPublicDeclarationTest.cs +++ b/test/Microsoft.ML.CodeAnalyzer.Tests/Code/BestFriendOnPublicDeclarationTest.cs @@ -52,7 +52,8 @@ public void BestFriendOnPublicDeclaration() diag.CreateDiagnosticResult(29, 10, "PublicClass"), diag.CreateDiagnosticResult(35, 6, "PublicStruct"), diag.CreateDiagnosticResult(40, 6, "PublicEnum"), - diag.CreateDiagnosticResult(47, 6, "PublicInterface") + diag.CreateDiagnosticResult(47, 6, "PublicInterface"), + diag.CreateDiagnosticResult(102, 10, "PublicMethod") }; VerifyDiagnosticResults(diags, analyzer, expected); diff --git a/test/Microsoft.ML.CodeAnalyzer.Tests/Resources/BestFriendOnPublicDeclaration.cs b/test/Microsoft.ML.CodeAnalyzer.Tests/Resources/BestFriendOnPublicDeclaration.cs index 882c499ec3..1515f21e1f 100644 --- a/test/Microsoft.ML.CodeAnalyzer.Tests/Resources/BestFriendOnPublicDeclaration.cs +++ b/test/Microsoft.ML.CodeAnalyzer.Tests/Resources/BestFriendOnPublicDeclaration.cs @@ -94,4 +94,14 @@ internal enum InternalEnum internal interface InternalInterface { } + + // this should fail the diagnostic + // a repro for https://github.com/dotnet/machinelearning/pull/2434#discussion_r254770946 + internal class InternalClassWithPublicMember + { + [BestFriend] + public void PublicMethod() + { + } + } }