Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Roslyn Analyzer project #349

Merged
merged 7 commits into from
May 24, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
namespace Carter.Analyzers;

using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
internal sealed class CarterModuleShouldNotHaveDependenciesAnalyzer : DiagnosticAnalyzer
{
public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterCompilationStartAction(OnCompilationStart);
}

private static void OnCompilationStart(CompilationStartAnalysisContext context)
{
var carterModuleType = context.Compilation.GetTypeByMetadataName("Carter.ICarterModule");
if (carterModuleType is null)
{
return;
}

context.RegisterSymbolAction(ctx => OnTypeAnalysis(ctx, carterModuleType), SymbolKind.NamedType);
}

private static void OnTypeAnalysis(SymbolAnalysisContext context, INamedTypeSymbol carterModuleType)
{
var typeSymbol = (INamedTypeSymbol)context.Symbol;
if (!typeSymbol.Interfaces.Contains(carterModuleType, SymbolEqualityComparer.Default))
{
return;
}

foreach (var constructor in typeSymbol.Constructors)
{
if (constructor.DeclaredAccessibility == Accessibility.Private || constructor.Parameters.Length == 0)
{
continue;
}

foreach (var syntaxReference in constructor.DeclaringSyntaxReferences)
{
var node = syntaxReference.GetSyntax();
SyntaxToken identifier;
if (node is ConstructorDeclarationSyntax constructorDeclaration)
{
identifier = constructorDeclaration.Identifier;
} else if (node is RecordDeclarationSyntax recordDeclaration)
{
identifier = recordDeclaration.Identifier;
}
else
{
continue;
}

var diagnostic = Diagnostic.Create(
DiagnosticDescriptors.CarterModuleShouldNotHaveDependencies,
identifier.GetLocation(),
identifier.Text
);
context.ReportDiagnostic(diagnostic);
}
}
}

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = [DiagnosticDescriptors.CarterModuleShouldNotHaveDependencies];
}
16 changes: 16 additions & 0 deletions src/Carter/Analyzers/DiagnosticDescriptors.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
using Microsoft.CodeAnalysis;

namespace Carter.Analyzers;

internal static class DiagnosticDescriptors
{
public static readonly DiagnosticDescriptor CarterModuleShouldNotHaveDependencies = new(
"CARTER1",
"Carter module should not have dependencies",
"'{0}' should not have dependencies",
"Usage",
DiagnosticSeverity.Warning,
true,
"When a class implements ICarterModule, it should not have any dependencies. This is because Carter uses minimal APIs and dependencies should be declared in the request delegate."
);
}
8 changes: 7 additions & 1 deletion src/Carter/Carter.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,27 @@
<AllowedOutputExtensionsInPackageBuildOutputFolder>$(AllowedOutputExtensionsInPackageBuildOutputFolder);.pdb</AllowedOutputExtensionsInPackageBuildOutputFolder>
<MinVerSkip Condition="'$(Configuration)' == 'Debug'">true</MinVerSkip>
<PackageReadmeFile>README.md</PackageReadmeFile>
<EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules>
<NoWarn>RS2008</NoWarn>
</PropertyGroup>
<ItemGroup>
<None Include="..\..\media\carterlogo.png" Pack="true" PackagePath="\" />
<None Include="..\..\README.md" Pack="true" PackagePath="\"/>
<None Include="$(OutputPath)\$(AssemblyName).dll" Pack="true" PackagePath="analyzers/dotnet/cs" Visible="false" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excuse my ignorance again - what does this do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't hold back with questions – it's important you understand the code I added :)

To answer your question: It's not enough to add the code to the project. To make analyzers work inside a NuGet package, they must be placed in a specific directory inside the package, in this case the analyzers/dotnet/cs directory, since it is a C# analyzer.

</ItemGroup>
<ItemGroup>
<FrameworkReference Include="Microsoft.AspNetCore.App" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="FluentValidation" Version="11.8.0" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.7.0" PrivateAssets="compile" />
<PackageReference Include="Microsoft.Extensions.DependencyModel" Version="8.0.0" />
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1" PrivateAssets="all" />
<PackageReference Include="MinVer" Version="2.5.0">
<PrivateAssets>all</PrivateAssets>
</PackageReference>

</ItemGroup>
<ItemGroup>
<InternalsVisibleTo Include="$(AssemblyName).Tests" />
</ItemGroup>
</Project>
2 changes: 2 additions & 0 deletions src/Carter/DependencyContextAssemblyCatalog.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ private static Assembly SafeLoadAssembly(AssemblyName assemblyName)
{
try
{
#pragma warning disable RS1035
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add this to the NoWarn csproj list?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, since this warning would be valid if DependencyContextAssemblyCatalog were an analyzer. Since it is not, the warning can be suppressed. Just one small caveat of not splitting up library code and analyzers.

return Assembly.Load(assemblyName);
#pragma warning restore RS1035
}
catch (Exception)
{
Expand Down
2 changes: 2 additions & 0 deletions src/Carter/ModelBinding/BindExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,14 @@ public static async Task BindAndSaveFile(this HttpRequest request, string saveLo

private static async Task SaveFileInternal(IFormFile file, string saveLocation, string fileName = "")
{
#pragma warning disable RS1035
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add this to the NoWarn csproj list?

if (!Directory.Exists(saveLocation))
Directory.CreateDirectory(saveLocation);

fileName = !string.IsNullOrWhiteSpace(fileName) ? fileName : file.FileName;

using (var fileToSave = File.Create(Path.Combine(saveLocation, fileName)))
await file.CopyToAsync(fileToSave);
#pragma warning restore RS1035
}
}
Loading
Loading