squid:S1312 naming scheme - is it not questionable?


(Trejkaz) #1

Most of the stuff in squid:S1312 I do agree with, but there is this one thing about the naming which I do have to question.

“Rename the “logger” logger to comply with the format “LOG(?:GER)?”.”

But here’s the thing:

  1. Our style guidelines say only to name actual constants in uppercase. As this is an object, and not a string, it cannot possibly be a constant, so uppercase would be incorrect by our own guidelines.
  2. Expanding the semantics to Google’s rules, one might also allow uppercase for immutable objects (we’re considering doing this too.) But Logger has setter methods, so clearly it is not immutable.

Therefore, I would lean towards the exact opposite naming to what is suggested here. If you defined “LOGGER”, you should get the warning. If you define “logger”, you should not get the warning.

Is this just a case of a “bad default” where we can fix the config ourselves? It isn’t immediately apparent how to get from a warning to the place it’s configured.

(Note: We’re using Log4J, whose loggers are mutable. Maybe there is another API where this is not the case, and in that case, under at least the Google rules, you could name it in uppercase.)

(Nicolas Peru) #2

The default value can be configured in the rule in your quality profile. So for this case, up to you to disagree with the default and change it.

I am noting and propagating the feedback about configuring the rule from the warning.