Skip to content

Commit af626e9

Browse files
Bart KoelmanBart Koelman
Bart Koelman
authored and
Bart Koelman
committed
AV1554: Suppress when implementing interface or overriding base method from external assembly
1 parent 4000015 commit af626e9

File tree

5 files changed

+182
-18
lines changed

5 files changed

+182
-18
lines changed

‎src/CSharpGuidelinesAnalyzer/CSharpGuidelinesAnalyzer.Test/Specs/Maintainability/DoNotUseOptionalParameterInTypeHierarchySpecs.cs‎

Lines changed: 80 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,34 @@ class C : I
8585
"Method 'C.M(int)' contains optional parameter 'q'");
8686
}
8787

88+
[Fact]
89+
internal void When_using_optional_parameter_in_implicitly_implemented_method_from_external_assembly_it_must_be_skipped()
90+
{
91+
// Arrange
92+
ParsedSourceCode source = new TypeSourceCodeBuilder()
93+
.WithReferenceToExternalAssemblyFor(@"
94+
public interface I
95+
{
96+
void M(int p = 5);
97+
}
98+
")
99+
.InGlobalScope(@"
100+
abstract class B : I
101+
{
102+
public abstract void M(int q = 8);
103+
}
104+
105+
class C : I
106+
{
107+
public void M(int q = 8) => throw null;
108+
}
109+
")
110+
.Build();
111+
112+
// Act and assert
113+
VerifyGuidelineDiagnostic(source);
114+
}
115+
88116
[Fact]
89117
internal void When_using_optional_parameter_in_explicitly_implemented_interface_method_it_must_be_reported()
90118
{
@@ -109,23 +137,46 @@ class C : I
109137
"Method 'C.I.M(int)' contains optional parameter 'q'");
110138
}
111139

140+
[Fact]
141+
internal void When_using_optional_parameter_in_explicitly_implemented_interface_method_from_external_assembly_it_must_be_skipped()
142+
{
143+
// Arrange
144+
ParsedSourceCode source = new TypeSourceCodeBuilder()
145+
.WithReferenceToExternalAssemblyFor(@"
146+
public interface I
147+
{
148+
void M(int p = 5);
149+
}
150+
")
151+
.InGlobalScope(@"
152+
class C : I
153+
{
154+
void I.M(int q = 8) => throw null;
155+
}
156+
")
157+
.Build();
158+
159+
// Act and assert
160+
VerifyGuidelineDiagnostic(source);
161+
}
162+
112163
[Fact]
113164
internal void When_using_optional_parameter_in_abstract_virtual_or_overridden_method_it_must_be_reported()
114165
{
115166
// Arrange
116167
ParsedSourceCode source = new TypeSourceCodeBuilder()
117168
.InGlobalScope(@"
118-
abstract class AbstractBase
169+
abstract class A
119170
{
120171
protected abstract void M([|int p = 5|]);
121172
}
122173
123-
class ConcreteBase
174+
class B
124175
{
125176
protected virtual void M([|int q = 8|]) => throw null;
126177
}
127178
128-
class ConcreteDerived : ConcreteBase
179+
class D : B
129180
{
130181
protected override void M([|int r = 12|]) => throw null;
131182
}
@@ -134,9 +185,32 @@ class ConcreteDerived : ConcreteBase
134185

135186
// Act and assert
136187
VerifyGuidelineDiagnostic(source,
137-
"Method 'AbstractBase.M(int)' contains optional parameter 'p'",
138-
"Method 'ConcreteBase.M(int)' contains optional parameter 'q'",
139-
"Method 'ConcreteDerived.M(int)' contains optional parameter 'r'");
188+
"Method 'A.M(int)' contains optional parameter 'p'",
189+
"Method 'B.M(int)' contains optional parameter 'q'",
190+
"Method 'D.M(int)' contains optional parameter 'r'");
191+
}
192+
193+
[Fact]
194+
internal void When_using_optional_parameter_in_overridden_method_from_external_assembly_it_must_be_skipped()
195+
{
196+
// Arrange
197+
ParsedSourceCode source = new TypeSourceCodeBuilder()
198+
.WithReferenceToExternalAssemblyFor(@"
199+
public abstract class B
200+
{
201+
protected abstract void M(int p = 5);
202+
}
203+
")
204+
.InGlobalScope(@"
205+
class C : B
206+
{
207+
protected override void M(int q = 8) => throw null;
208+
}
209+
")
210+
.Build();
211+
212+
// Act and assert
213+
VerifyGuidelineDiagnostic(source);
140214
}
141215

142216
protected override DiagnosticAnalyzer CreateAnalyzer()

‎src/CSharpGuidelinesAnalyzer/CSharpGuidelinesAnalyzer.Test/TestDataBuilders/SourceCodeBuilder.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,16 @@ namespace CSharpGuidelinesAnalyzer.Test.TestDataBuilders
1515
/// <summary />
1616
internal abstract class SourceCodeBuilder : ITestDataBuilder<ParsedSourceCode>
1717
{
18-
[NotNull]
19-
private static readonly AnalyzerTestContext DefaultTestContext = new AnalyzerTestContext(string.Empty, Array.Empty<TextSpan>(), LanguageNames.CSharp,
20-
new AnalyzerOptions(ImmutableArray<AdditionalText>.Empty));
21-
2218
[ItemNotNull]
2319
protected static readonly ImmutableArray<string> DefaultNamespaceImports = new[]
2420
{
2521
"System"
2622
}.ToImmutableArray();
2723

24+
[NotNull]
25+
public static readonly AnalyzerTestContext DefaultTestContext = new AnalyzerTestContext(string.Empty, Array.Empty<TextSpan>(), LanguageNames.CSharp,
26+
new AnalyzerOptions(ImmutableArray<AdditionalText>.Empty));
27+
2828
[NotNull]
2929
[ItemNotNull]
3030
private readonly HashSet<string> namespaceImports;

‎src/CSharpGuidelinesAnalyzer/CSharpGuidelinesAnalyzer.Test/TestDataBuilders/SourceCodeBuilderExtensions.cs‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
using System.Collections.Immutable;
2+
using System.IO;
3+
using System.Linq;
24
using System.Reflection;
5+
using FluentAssertions;
36
using JetBrains.Annotations;
47
using Microsoft.CodeAnalysis;
8+
using Microsoft.CodeAnalysis.CSharp;
59
using Microsoft.CodeAnalysis.Diagnostics;
10+
using Microsoft.CodeAnalysis.Emit;
611
using RoslynTestFramework;
712

813
namespace CSharpGuidelinesAnalyzer.Test.TestDataBuilders
@@ -66,6 +71,54 @@ public static TBuilder WithReference<TBuilder>([NotNull] this TBuilder source, [
6671
return source;
6772
}
6873

74+
[NotNull]
75+
public static TBuilder WithReferenceToExternalAssemblyFor<TBuilder>([NotNull] this TBuilder source, [NotNull] string code)
76+
where TBuilder : SourceCodeBuilder
77+
{
78+
Guard.NotNull(source, nameof(source));
79+
Guard.NotNull(code, nameof(code));
80+
81+
Stream assemblyStream = GetInMemoryAssemblyStreamForCode(code, "TempAssembly");
82+
PortableExecutableReference reference = MetadataReference.CreateFromStream(assemblyStream);
83+
84+
source.Editor.UpdateTestContext(context =>
85+
{
86+
ImmutableHashSet<MetadataReference> references = context.References.Add(reference);
87+
return context.WithReferences(references);
88+
});
89+
90+
return source;
91+
}
92+
93+
[NotNull]
94+
private static Stream GetInMemoryAssemblyStreamForCode([NotNull] string code, [NotNull] string assemblyName,
95+
[NotNull] [ItemNotNull] params MetadataReference[] references)
96+
{
97+
SyntaxTree tree = CSharpSyntaxTree.ParseText(code);
98+
ImmutableArray<SyntaxTree> trees = ImmutableArray.Create(tree);
99+
var options = new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary);
100+
101+
CSharpCompilation compilation = CSharpCompilation.Create(assemblyName, trees).WithOptions(options);
102+
compilation = compilation.AddReferences(SourceCodeBuilder.DefaultTestContext.References);
103+
compilation = compilation.AddReferences(references);
104+
105+
var stream = new MemoryStream();
106+
107+
EmitResult emitResult = compilation.Emit(stream);
108+
ValidateCompileErrors(emitResult);
109+
110+
stream.Seek(0, SeekOrigin.Begin);
111+
112+
return stream;
113+
}
114+
115+
private static void ValidateCompileErrors([NotNull] EmitResult emitResult)
116+
{
117+
Diagnostic[] compilerErrors = emitResult.Diagnostics.Where(diagnostic => diagnostic.Severity == DiagnosticSeverity.Error).ToArray();
118+
compilerErrors.Should().BeEmpty("external assembly should not have compile errors");
119+
emitResult.Success.Should().BeTrue();
120+
}
121+
69122
[NotNull]
70123
public static TBuilder WithDocumentationComments<TBuilder>([NotNull] this TBuilder source)
71124
where TBuilder : SourceCodeBuilder

‎src/CSharpGuidelinesAnalyzer/CSharpGuidelinesAnalyzer.Test/TestDataBuilders/XmlSettingsBuilder.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,7 @@ public FakeAdditionalText([NotNull] string content)
4545
}
4646

4747
[NotNull]
48-
#pragma warning disable AV1554 // Method contains optional parameter in type hierarchy
4948
public override SourceText GetText(CancellationToken cancellationToken = new CancellationToken())
50-
#pragma warning restore AV1554 // Method contains optional parameter in type hierarchy
5149
{
5250
return sourceText;
5351
}

‎src/CSharpGuidelinesAnalyzer/CSharpGuidelinesAnalyzer/Rules/Maintainability/DoNotUseOptionalParameterInTypeHierarchyAnalyzer.cs‎

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,19 +46,58 @@ private static void AnalyzeParameter(SymbolAnalysisContext context)
4646
if (parameter.IsOptional)
4747
{
4848
INamedTypeSymbol type = parameter.ContainingType;
49-
ISymbol method = parameter.ContainingSymbol;
49+
50+
if (!(parameter.ContainingSymbol is IMethodSymbol method))
51+
{
52+
return;
53+
}
5054

5155
if (type.TypeKind == TypeKind.Interface || method.IsInterfaceImplementation() || method.IsAbstract || method.IsVirtual || method.IsOverride)
5256
{
53-
string containerName = method.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat);
57+
if (!IsOverrideFromExternalAssembly(method) && !IsInterfaceImplementationFromExternalAssembly(method))
58+
{
59+
string containerName = method.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat);
60+
61+
SyntaxReference syntaxReference = parameter.DeclaringSyntaxReferences.First();
62+
var location = Location.Create(syntaxReference.SyntaxTree, syntaxReference.Span);
63+
64+
var diagnostic = Diagnostic.Create(Rule, location, containerName, parameter.Name);
65+
context.ReportDiagnostic(diagnostic);
66+
}
67+
}
68+
}
69+
}
70+
71+
private static bool IsOverrideFromExternalAssembly([NotNull] IMethodSymbol method)
72+
{
73+
IMethodSymbol baseMethod = method.OverriddenMethod;
74+
75+
while (baseMethod != null)
76+
{
77+
if (!baseMethod.ContainingAssembly.Equals(method.ContainingAssembly))
78+
{
79+
return true;
80+
}
5481

55-
SyntaxReference syntaxReference = parameter.DeclaringSyntaxReferences.First();
56-
var location = Location.Create(syntaxReference.SyntaxTree, syntaxReference.Span);
82+
baseMethod = baseMethod.OverriddenMethod;
83+
}
84+
85+
return false;
86+
}
5787

58-
var diagnostic = Diagnostic.Create(Rule, location, containerName, parameter.Name);
59-
context.ReportDiagnostic(diagnostic);
88+
private static bool IsInterfaceImplementationFromExternalAssembly([NotNull] IMethodSymbol method)
89+
{
90+
foreach (ISymbol interfaceMethod in method.ContainingType.AllInterfaces.SelectMany(@interface => @interface.GetMembers()))
91+
{
92+
ISymbol implementer = method.ContainingType.FindImplementationForInterfaceMember(interfaceMethod);
93+
94+
if (method.Equals(implementer))
95+
{
96+
return !method.ContainingAssembly.Equals(interfaceMethod.ContainingAssembly);
6097
}
6198
}
99+
100+
return false;
62101
}
63102
}
64103
}

0 commit comments

Comments
 (0)