search

SonarSource / sonar-dotnet

Code analyzer for C# and VB.NET projects
https://redirect.sonarsource.com/plugins/csharp.html
GNU Lesser General Public License v3.0
787 stars 227 forks source link

Update S1200: Rule should ignore 'nameof()' references #2123

Closed saciervo closed 5 years ago

saciervo commented 5 years ago

Description

Using the nameof() keyword operator counts against the threshold of Rule S1200.

While it is technically true that a type used as a nameof() parameter will be coupled to the class you are writing, I think it is not a problematic dependency because it is only about the name of the type and not about behavior or responsibility.

Repro steps

Create a class and write some code containing several nameof()'s that point to several other classes and namespaces than the one you are writing.

Expected behavior

The analyzer should not demand to split the class into smaller and more specialized ones.

Actual behavior

The analyzer demands to split the class into smaller and more specialized ones.

Known workarounds

None

Related information

Evangelink commented 5 years ago

Hi @saciervo,

Thank you for the feedback. You are right! I still think it would be worth somehow to capture the kind of link/dependency created through nameof.

Out of curiosity, would you mind sharing some use cases where you would need to reference a class or method name without having a behavioral dependency on it?

saciervo commented 5 years ago

Hi @Evangelink

Sorry for the late reply. The problem occured in a method that generates an SQL command for multiple tables. Usually we use EntityFramework for database access, but in this special case we needed raw SQL. Instead of hard coding the table names, my co-worker used nameof() with the entities. The method is not that large and has even less complexity (your typical StringBuilder scenario), so she was baffled by this message from the analyzer and asked me why that is.

It's not a particularly strong case, but I just couldn't see what the analyzer had to complain about this method at first sight. It was only at second glance that I suspected what might be the reason. Then I thought I should create an issue for this, because for me and her, it was just not obvious.