audaciaconsulting / audacia.codeanalysis

Overview

The Audacia.CodeAnalysis repo contains static code analysis configuration and analyzers.

The dotnet-roslyn folder contains:

• Example .editorconfig files - see dotnet-roslyn/config
• Custom Roslyn analyzers and helpers - see dotnet-roslyn/analyzers

The eslint folder contains:

• Some default eslint config - see eslint/config/audacia-eslint-config

Contributing

We welcome contributions! Please feel free to check our Contribution Guidlines for feature requests, issue reporting and guidelines.

audacia.codeanalysis's Issues

Rules RCS1070 and IDE0010 contradict each other

The warning IDE0010 asks you to cover all cases in a given switch statement.

Conversely, RCS1070 asks to remove redundant default cases.

Obviously this is only if your default case is redundant, however there is an open issue on roslynator for this.

In my opinion, IDE0010 should be a suggestion in the base .editorconfig, but happy to discuss alternatives.

ACL1002 Warning added to a class with primary constructor

Adding a primary constructor to a class seems to make the compiler think it's a big method instead of a class:

It would be preferable if this warning was supressed on class definitions. (note: this may now also start showing the more than X arguments warning, which also would be nice if disabled against a class? I think a similar idea was raised against primary constructors on records).

Update links in README's to point to GitHub references instead of Azure DevOps

https://github.com/audaciaconsulting/Audacia.CodeAnalysis/blob/master/dotnet-roslyn/config/README.md
https://github.com/audaciaconsulting/Audacia.CodeAnalysis/blob/master/dotnet-roslyn/config/Audacia.CodeAnalysis/README.md

Will be worth going through the other README's to find any additional broken links.

MaxParameterCountAttribute cannot be used on classes with primary constructors

The MaxParameterCountAttribute is only valid on AttributeTargets.Constructor | AttributeTargets.Method so cannot be applied to a class that has a primary constructor

The attribute should also support AttributeTargets.Class, assuming this is correctly picked up by the analyzer, increasing the max count allowed. If not, support for that will also need to be added.

Feature request: analyser that enforces CS-02.9: "Use records for types that just contain data"

From the standard:

Use records for DTOs and other types that encapsulate data rather than behaviour, and where value-based equality makes sense. They should generally be immutable, although records can contain mutable properties.

Create an analyser that checks whether objects that contain data are record types, rather than a class. This should exclude interfaces.

In terms of how, I think we should have a configurable file suffix (that may or may not default to just *Dto), because conventions between projects may vary (e.g *Request / *Result).

Also provide an auto-fix for this.

Feature request: ACL analyser for including numbers in variables

We currently suppress AV1706 in favour of ACL1004 as it supports a wider list of abbreviations.

In a similar vein, I propose a new analyzer that would replace AV1704 with a configurable list of allowed numbers, as there are some numbers that are meaningful to domain without having to type out OneThousandAndTwentyThree (or whatever) for our variable names.

@audacia/eslint-config - Angular 17

@audacia/eslint-config is not compatible with Angular ~17 because it references older versions of typescript-eslint/eslint-plugin and typescript-eslint/parser. I managed get around by having two versions of the said packages in the package.json like below

"@typescript-eslint/eslint-plugin": "^6.19.0 || ^5.13.0",
"@typescript-eslint/parser": "^6.19.0 || ^5.0.0",

SA1010 reporting on incorrect spacing

SA1010 says there should not be a space here, but there should be:

If you apply the fix, SA1002 and IDE0055 both report an issue:

Think this is because the version of StyleCop.Analyzers referenced in Audacia.CodeAnalysis is not aware of a lot of C#11 features.

There is a somewhat related issue here: DotNetAnalyzers/StyleCopAnalyzers#3503

Updating StyleCop.Analyzers locally to 1.2.0-beta.556 fixes the issue:

I have had a skim through the changelogs of StyleCop.Analyzers and cannot see any big changes, mostly seems like tweaks to existing rules in order to support newer C# features, so should be an easy upgrade

ACL1010 highlights the whole file as a warning

If the project you are in does not have nullable reference types enabled, then ACL1010 highlights every line in every file:

Pretty sure this is because the location of the diagnostic is being set to the root of the syntax tree, i.e. the whole file

Audacia.CodeAnalysis/dotnet-roslyn/analyzers/Audacia.CodeAnalysis.Analyzers/src/Audacia.CodeAnalysis.Analyzers/Rules/NullableReferenceTypes/NullableReferenceTypesAnalyzer.cs

Lines 71 to 76 in 18b5bb4

	var location = context.SemanticModel
	.SyntaxTree
	.GetRoot()
	.GetLocation();
	var diagnostic = Diagnostic.Create(Rule, location);

Instead, I think the location can be set a Location.None to indicate the whole file has an issue.

For example, this is how AV2210 is reported - only once in the whole file:

And here is how the diagnostic for it is created:

https://github.com/bkoelman/CSharpGuidelinesAnalyzer/blob/e84590e1b0c0682a9a932d6fdd3f4ac1637cd730/src/CSharpGuidelinesAnalyzer/CSharpGuidelinesAnalyzer/Rules/Framework/BuildWithTheHighestWarningLevelAnalyzer.cs#L46