diff --git a/Directory.Packages.props b/Directory.Packages.props index 972622cab02..45c868a31fb 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -32,13 +32,13 @@ - + - + - + @@ -49,8 +49,7 @@ - - + diff --git a/src/Analyzers/Razor.Diagnostics.Analyzers.Test/Razor.Diagnostics.Analyzers.Test.csproj b/src/Analyzers/Razor.Diagnostics.Analyzers.Test/Razor.Diagnostics.Analyzers.Test.csproj index d8977ed8c0f..ab94ba1cf89 100644 --- a/src/Analyzers/Razor.Diagnostics.Analyzers.Test/Razor.Diagnostics.Analyzers.Test.csproj +++ b/src/Analyzers/Razor.Diagnostics.Analyzers.Test/Razor.Diagnostics.Analyzers.Test.csproj @@ -8,8 +8,7 @@ - - + diff --git a/src/Analyzers/Razor.Diagnostics.Analyzers.Test/Verifiers/CSharpAnalyzerVerifier`1+Test.cs b/src/Analyzers/Razor.Diagnostics.Analyzers.Test/Verifiers/CSharpAnalyzerVerifier`1+Test.cs index d59cd6bfe2e..f01ea1f7665 100644 --- a/src/Analyzers/Razor.Diagnostics.Analyzers.Test/Verifiers/CSharpAnalyzerVerifier`1+Test.cs +++ b/src/Analyzers/Razor.Diagnostics.Analyzers.Test/Verifiers/CSharpAnalyzerVerifier`1+Test.cs @@ -3,14 +3,14 @@ using Microsoft.CodeAnalysis.CSharp.Testing; using Microsoft.CodeAnalysis.Diagnostics; -using Microsoft.CodeAnalysis.Testing.Verifiers; +using Microsoft.CodeAnalysis.Testing; namespace Razor.Diagnostics.Analyzers.Test; public static partial class CSharpAnalyzerVerifier where TAnalyzer : DiagnosticAnalyzer, new() { - public class Test : CSharpAnalyzerTest + public class Test : CSharpAnalyzerTest { public Test() { diff --git a/src/Analyzers/Razor.Diagnostics.Analyzers.Test/Verifiers/CSharpAnalyzerVerifier`1.cs b/src/Analyzers/Razor.Diagnostics.Analyzers.Test/Verifiers/CSharpAnalyzerVerifier`1.cs index a24421ec5b5..56194eec436 100644 --- a/src/Analyzers/Razor.Diagnostics.Analyzers.Test/Verifiers/CSharpAnalyzerVerifier`1.cs +++ b/src/Analyzers/Razor.Diagnostics.Analyzers.Test/Verifiers/CSharpAnalyzerVerifier`1.cs @@ -7,7 +7,6 @@ using Microsoft.CodeAnalysis.CSharp.Testing; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Testing; -using Microsoft.CodeAnalysis.Testing.Verifiers; namespace Razor.Diagnostics.Analyzers.Test; @@ -16,15 +15,15 @@ public static partial class CSharpAnalyzerVerifier { /// public static DiagnosticResult Diagnostic() - => CSharpAnalyzerVerifier.Diagnostic(); + => CSharpAnalyzerVerifier.Diagnostic(); /// public static DiagnosticResult Diagnostic(string diagnosticId) - => CSharpAnalyzerVerifier.Diagnostic(diagnosticId); + => CSharpAnalyzerVerifier.Diagnostic(diagnosticId); /// public static DiagnosticResult Diagnostic(DiagnosticDescriptor descriptor) - => CSharpAnalyzerVerifier.Diagnostic(descriptor); + => CSharpAnalyzerVerifier.Diagnostic(descriptor); /// public static async Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected) diff --git a/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Analyzers/AnalyzerIDs.cs b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Analyzers/AnalyzerIDs.cs new file mode 100644 index 00000000000..1c4fdb47bda --- /dev/null +++ b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Analyzers/AnalyzerIDs.cs @@ -0,0 +1,9 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.CodeAnalysis.Razor.Compiler.Analyzers; + +internal static class AnalyzerIDs +{ + internal const string ComponentParameterNullableWarningSuppressionId = "RZS1001"; +} diff --git a/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Analyzers/AnalyzerResources.resx b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Analyzers/AnalyzerResources.resx new file mode 100644 index 00000000000..83c248dcd6b --- /dev/null +++ b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Analyzers/AnalyzerResources.resx @@ -0,0 +1,123 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + Suppress CS8618 when a component parameter is marked with Microsoft.AspNetCore.Components.EditorRequiredAttribute + + \ No newline at end of file diff --git a/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Analyzers/ComponentParameterNullableWarningSuppressor.cs b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Analyzers/ComponentParameterNullableWarningSuppressor.cs new file mode 100644 index 00000000000..f9c28d43fb9 --- /dev/null +++ b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Analyzers/ComponentParameterNullableWarningSuppressor.cs @@ -0,0 +1,80 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.CodeAnalysis.Razor.Compiler.Analyzers; + +#pragma warning disable RS1041 // Compiler extensions should be implemented in assemblies targeting netstandard2.0 + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class ComponentParameterNullableWarningSuppressor : DiagnosticSuppressor +{ + private static readonly LocalizableString Description = new LocalizableResourceString(nameof(AnalyzerResources.ComponentParameterNullableWarningSuppressorDescription), AnalyzerResources.ResourceManager, typeof(AnalyzerResources)); + + //Suppress CS8618: "Non-nullable {0} '{1}' must contain a non-null value when exiting constructor. Consider declaring the {0} as nullable." + public override ImmutableArray SupportedSuppressions => [ + new SuppressionDescriptor(AnalyzerIDs.ComponentParameterNullableWarningSuppressionId, "CS8618", Description) + ]; + + public override void ReportSuppressions(SuppressionAnalysisContext context) + { + var editorRequiredSymbol = context.Compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Components.EditorRequiredAttribute"); + var parameterSymbol = context.Compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Components.ParameterAttribute"); + if (parameterSymbol is null || editorRequiredSymbol is null) + { + return; + } + + foreach (var diagnostic in context.ReportedDiagnostics) + { + var node = diagnostic.Location.SourceTree?.GetRoot(context.CancellationToken).FindNode(diagnostic.Location.SourceSpan); + if (node is PropertyDeclarationSyntax propertySyntax && propertySyntax.AttributeLists.Any()) + { + var symbol = context.GetSemanticModel(propertySyntax.SyntaxTree).GetDeclaredSymbol(propertySyntax, context.CancellationToken); + if (IsValidEditorRequiredParameter(symbol)) + { + context.ReportSuppression(Suppression.Create(SupportedSuppressions[0], diagnostic)); + } + } + } + + bool IsValidEditorRequiredParameter(ISymbol? symbol) + { + // public instance property, with a public setter + if (symbol is not IPropertySymbol { DeclaredAccessibility: Accessibility.Public, IsStatic: false, SetMethod: not null and { DeclaredAccessibility: Accessibility.Public } }) + { + return false; + } + + // has both [Parameter] and [EditorRequired] attributes + bool hasParameter = false, hasRequired = false; + foreach (var attribute in symbol.GetAttributes()) + { + if (!hasParameter && SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, parameterSymbol)) + { + hasParameter = true; + if (hasRequired) + { + break; + } + continue; + } + + if (!hasRequired && SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, editorRequiredSymbol)) + { + hasRequired = true; + if (hasParameter) + { + break; + } + continue; + } + } + return hasParameter && hasRequired; + } + } +} + diff --git a/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Microsoft.CodeAnalysis.Razor.Compiler.csproj b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Microsoft.CodeAnalysis.Razor.Compiler.csproj index 6428e0f55ed..2b3d5eb80e3 100644 --- a/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Microsoft.CodeAnalysis.Razor.Compiler.csproj +++ b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Microsoft.CodeAnalysis.Razor.Compiler.csproj @@ -1,4 +1,4 @@ - + Razor is a markup syntax for adding server-side logic to web pages. This package contains the Razor compiler. @@ -24,6 +24,7 @@ + diff --git a/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.Tests/ComponentParameterNullableWarningSuppressorTests.cs b/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.Tests/ComponentParameterNullableWarningSuppressorTests.cs new file mode 100644 index 00000000000..1d373e52485 --- /dev/null +++ b/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.Tests/ComponentParameterNullableWarningSuppressorTests.cs @@ -0,0 +1,279 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Immutable; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Razor.Compiler.Analyzers; +using Microsoft.CodeAnalysis.Testing; +using Xunit; + +namespace Microsoft.CodeAnalysis.Razor.Analyzers.Tests +{ + public class ComponentParameterNullableWarningSuppressorTests + { + [Fact] + public async Task ParameterEditorRequiredNoWarning() + { + var testCode = """ + #nullable enable + using System; + using Microsoft.AspNetCore.Components; + + public class MyComponent : ComponentBase + { + [Parameter, EditorRequired] + public string MyParameter { get; set; } + } + """; + + await VerifyAnalyzerAsync(testCode, + // /0/Test0.cs(9,19): warning CS8618: Non-nullable property 'MyParameter' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable. + DiagnosticResult.CompilerWarning("CS8618").WithSpan(8, 19, 8, 30).WithSpan(8, 19, 8, 30).WithArguments("property", "MyParameter").WithIsSuppressed(true) + ); + } + + [Fact] + public async Task NoEditorRequiredStillReports() + { + var testCode = """ + #nullable enable + using System; + using Microsoft.AspNetCore.Components; + + public class MyComponent : ComponentBase + { + [Parameter] + public string MyParameter { get; set; } + } + """; + + await VerifyAnalyzerAsync(testCode, + // /0/Test0.cs(9,19): warning CS8618: Non-nullable property 'MyParameter' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable. + DiagnosticResult.CompilerWarning("CS8618").WithSpan(8, 19, 8, 30).WithSpan(8, 19, 8, 30).WithArguments("property", "MyParameter") + ); + } + + [Fact] + public async Task NoParameterRequiredStillReports() + { + var testCode = """ + #nullable enable + using System; + using Microsoft.AspNetCore.Components; + + public class MyComponent : ComponentBase + { + [EditorRequired] + public string MyParameter { get; set; } + } + """; + + await VerifyAnalyzerAsync(testCode, + // /0/Test0.cs(9,19): warning CS8618: Non-nullable property 'MyParameter' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable. + DiagnosticResult.CompilerWarning("CS8618").WithSpan(8, 19, 8, 30).WithSpan(8, 19, 8, 30).WithArguments("property", "MyParameter") + ); + } + + [Fact] + public async Task AliasedAttributes() + { + var testCode = """ + #nullable enable + using System; + using Microsoft.AspNetCore.Components; + using MyParameter = Microsoft.AspNetCore.Components.ParameterAttribute; + using MyRequired = Microsoft.AspNetCore.Components.EditorRequiredAttribute; + + public class MyComponent : ComponentBase + { + [MyParameter, MyRequired] + public string MyParameter { get; set; } + } + """; + + await VerifyAnalyzerAsync(testCode, + // /0/Test0.cs(9,19): warning CS8618: Non-nullable property 'MyParameter' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable. + DiagnosticResult.CompilerWarning("CS8618").WithSpan(10, 19, 10, 30).WithSpan(10, 19, 10, 30).WithArguments("property", "MyParameter").WithIsSuppressed(true) + ); + } + + [Fact] + public async Task LocallyDefinedAttributes() + { + var testCode = """ + #nullable enable + using System; + using Microsoft.AspNetCore.Components; + + public class MyComponent + { + [Parameter, EditorRequired] + public string MyParameter { get; set; } + } + + namespace Microsoft.AspNetCore.Components + { + public class ParameterAttribute : Attribute { } + public class EditorRequiredAttribute : Attribute { } + } + + """; + + await VerifyAnalyzerAsync(testCode, extraReferences: [], + // /0/Test0.cs(9,19): warning CS8618: Non-nullable property 'MyParameter' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable. + DiagnosticResult.CompilerWarning("CS8618").WithSpan(8, 19, 8, 30).WithSpan(8, 19, 8, 30).WithArguments("property", "MyParameter").WithIsSuppressed(true) + ); + } + + [Fact] + public async Task LocallyDefinedAttributesDifferentNamespace() + { + var testCode = """ + #nullable enable + using System; + using MyNamespace; + + public class MyComponent + { + [Parameter, EditorRequired] + public string MyParameter { get; set; } + } + + namespace MyNamespace + { + public class ParameterAttribute : Attribute { } + public class EditorRequiredAttribute : Attribute { } + } + + """; + + await VerifyAnalyzerAsync(testCode, extraReferences: [], + // /0/Test0.cs(9,19): warning CS8618: Non-nullable property 'MyParameter' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable. + DiagnosticResult.CompilerWarning("CS8618").WithSpan(8, 19, 8, 30).WithSpan(8, 19, 8, 30).WithArguments("property", "MyParameter") + ); + } + + [Fact] + public async Task LocallyDefinedAttributesAndSdkAttributes() + { + var testCode = """ + #nullable enable + using System; + using Microsoft.AspNetCore.Components; + + public class MyComponent + { + [Parameter, EditorRequired] + public string MyParameter { get; set; } + } + + namespace Microsoft.AspNetCore.Components + { + public class ParameterAttribute : Attribute { } + public class EditorRequiredAttribute : Attribute { } + } + + """; + + await VerifyAnalyzerAsync(testCode, + // /0/Test0.cs(8,19): warning CS8618: Non-nullable property 'MyParameter' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable. + DiagnosticResult.CompilerWarning("CS8618").WithSpan(8, 19, 8, 30).WithSpan(8, 19, 8, 30).WithArguments("property", "MyParameter").WithIsSuppressed(true), + // /0/Test0.cs(7,6): warning CS0436: The type 'ParameterAttribute' in '/0/Test0.cs' conflicts with the imported type 'ParameterAttribute' in 'Microsoft.AspNetCore.Components, Version=8.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'. Using the type defined in '/0/Test0.cs'. + DiagnosticResult.CompilerWarning("CS0436").WithSpan(7, 6, 7, 15).WithArguments("/0/Test0.cs", "Microsoft.AspNetCore.Components.ParameterAttribute", "Microsoft.AspNetCore.Components, Version=8.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60", "Microsoft.AspNetCore.Components.ParameterAttribute"), + // /0/Test0.cs(7,17): warning CS0436: The type 'EditorRequiredAttribute' in '/0/Test0.cs' conflicts with the imported type 'EditorRequiredAttribute' in 'Microsoft.AspNetCore.Components, Version=8.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'. Using the type defined in '/0/Test0.cs'. + DiagnosticResult.CompilerWarning("CS0436").WithSpan(7, 17, 7, 31).WithArguments("/0/Test0.cs", "Microsoft.AspNetCore.Components.EditorRequiredAttribute", "Microsoft.AspNetCore.Components, Version=8.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60", "Microsoft.AspNetCore.Components.EditorRequiredAttribute") + ); + } + + [Theory] + [InlineData("internal")] + [InlineData("private")] + [InlineData("protected internal")] + [InlineData("protected")] + [InlineData("public static")] + public async Task IncorrectModifiersStillReport(string modifiers) + { + var testCode = $$""" + #nullable enable + using System; + using Microsoft.AspNetCore.Components; + + public class MyComponent : ComponentBase + { + [Parameter, EditorRequired] + {{modifiers}} + string MyParameter { get; set; } + } + """; + + await VerifyAnalyzerAsync(testCode, + // /0/Test0.cs(9,19): warning CS8618: Non-nullable property 'MyParameter' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable. + DiagnosticResult.CompilerWarning("CS8618").WithSpan(9, 12, 9, 23).WithSpan(9, 12, 9, 23).WithArguments("property", "MyParameter") + ); + } + + [Theory] + [InlineData("")] + [InlineData("init;")] + [InlineData("private set;")] + [InlineData("private init;")] + public async Task IncorrectSetterStillReport(string setter) + { + var testCode = $$""" + #nullable enable + using System; + using Microsoft.AspNetCore.Components; + + public class MyComponent : ComponentBase + { + [Parameter, EditorRequired] + public string MyParameter { get; {{setter}} } + } + """; + + await VerifyAnalyzerAsync(testCode, + // /0/Test0.cs(9,19): warning CS8618: Non-nullable property 'MyParameter' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable. + DiagnosticResult.CompilerWarning("CS8618").WithSpan(8, 19, 8, 30).WithSpan(8, 19, 8, 30).WithArguments("property", "MyParameter") + ); + } + + [Fact] + public async Task RequiredPropertyDoesNotReport() + { + var testCode = $$""" + #nullable enable + using System; + using Microsoft.AspNetCore.Components; + + public class MyComponent : ComponentBase + { + [Parameter, EditorRequired] + public required string MyParameter { get; set; } + } + """; + + await VerifyAnalyzerAsync(testCode); + } + + private static Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected) + => VerifyAnalyzerAsync(source, + Basic.Reference.Assemblies.AspNet80.References.All, + expected); + + private static async Task VerifyAnalyzerAsync(string source, ImmutableArray extraReferences, params DiagnosticResult[] expected) + { + var test = new CSharpAnalyzerTest + { + TestCode = source, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + CompilerDiagnostics = CompilerDiagnostics.Warnings, + DisabledDiagnostics = { "CS1591" }, // Missing XML comment for publicly visible type or member + }; + + test.TestState.AdditionalReferences.AddRange(extraReferences); + test.ExpectedDiagnostics.AddRange(expected); + await test.RunAsync(); + } + } +} diff --git a/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.Tests/Microsoft.NET.Sdk.Razor.SourceGenerators.Test.csproj b/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.Tests/Microsoft.NET.Sdk.Razor.SourceGenerators.Test.csproj index 581f9841034..8c47d77c2d9 100644 --- a/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.Tests/Microsoft.NET.Sdk.Razor.SourceGenerators.Test.csproj +++ b/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.Tests/Microsoft.NET.Sdk.Razor.SourceGenerators.Test.csproj @@ -23,12 +23,14 @@ + +