S6212: Always replacing variable with "var" may be confusing

Hello,

I got a lot of complaints around the S6212 rules. For instance, sonarcloud note this line as non compliant :

HttpHeaders httpHeaders = client.createOauthHeader(...);

The sonar compliant solution would be:

var httpHeaders = client.createOauthHeader(...);

but clearly I’m not a JVM and I’m not able to define the object type at first glance.

I would be 100% for relaxing this rules. like only check instanciation themselves : var obj=new MyClass(....); all other usage may confuse people.

Hello @GregoireW

I moved your question to a new topic, as the other one is an announcement, not the best place to discuss a specific rule.

I understand your concern, I agree that, even when it is possible, it is not always wise to use var.
This is why we tried to limit the scope of this rule to report an issue only when:

  • The initializer is “self-explained”.
MyClass myClass = new MyClass();

int i = 10;
  • The type is mentioned in the name.
MyClass myClass = get();
  • The type is mentioned in the initializer:
MyClass something = MyClass.getMyClass();

Your case enters the second point, it seems redundant to repeat the type when it is mentioned in the name.
In addition, as we agree that this is a question of taste, this is an “Info” code smell, with 0 cost, it will not impact the quality gate in any way. This rule is mainly to suggest a new coding style.

Do these additional details clarify the situation?

1 Like

Hello,

Thank you, I did not have in mind the difference “type mentioned in the name” / “random name”

It is much clearer now

Hey there,

just my two cents for this rule: It should be relaxed or even made optional / inactive in the default profile.
In most cases I feel it does neither help readability nor provide better understanding.
Further I disagree with your 2nd example. The term “self-explaining” can be pretty biased in my opinion:

int i = 10; // compliant because why not?
var i = 10; // not self-explaining but less readable IMHO
boolean checked = isChecked(); // compliant because why not?
var checked = isChecked(); // not self-explaining but less readable IMHO

Another suggestion I have is to make the rule aware of characters it reduces in code:

MySuperLongClassNameBtwIsThereEvenARuleForClassNameLenght instance = MySuperLongClassNameBtwIsThereEvenARuleForClassNameLength(); // oh god
var instance = MySuperLongClassNameBtwIsThereEvenARuleForClassNameLength(); // actually better

versus

Tool instance = Tool(); // 4 characters classname
var instance = Tool(); // saving 1 character but reducing readability

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.

For the record, we decided to remove it from the default quality profile. More details: SONARJAVA-3870.