Bugs are instances of faulty code, while code smells are code which runs OK, but impede maintenance for one reason or another. Some of the bug rules include checking for things that are just stupid without being necessarily flawed, on the assumption that the coder really intended something else. For example, java:S2252 catches loops where SQ can determine statically that the body will never be executed.
Some rules are tagged “performance” (around 30 for Java, for instance) and pertain to things that will cause perf hits, such as building up a String with += in a loop. I noticed that most of these are classified as code smells, although a few are bugs. Were there specific criteria for deciding which classification to use? It seems all the perf issues should be bugs, at least minor bugs, since they may cause unexpected behavior (unexpectedly slow).
In fact, there is a guideline, published here. As you’ll see, Code Smell rules are set largely by default.
And as you might expect, individual biases and opinions also come into it. While a rule for a performance issue with a potentially severe impact is more likely to be classified as a bug, the judgement of our language specialists is always in play.
Are there specific examples you think need re-examining?
OK thanks, I was just wondering. I see the two Java rules tagged performance that are bugs rather than smells also have other tags (leak or deadlock) which makes them sound worse than merely costing perf.
Perhaps you could make another issue category called Performance for things that just cause perf hits. Then you could use the severity to indicate how bad they might be.
In which case you could move some others into there, like java:S1157 which complains when you do foo.toLowerCase().equals(bar.LowerCase()) rather than foo.equalsIgnoreCase(bar). Right now that’s a minor smell tagged clumsy, but not perf.
I’m currently writing performance/eco-design rules for different languages. I agree they don’t really fit neither in “quality of code” nor in “security”.
I’m using “CODE_SMELL” since it looks like a convention for these rules for now.
Maybe the reasoning is something like: “if your code is slow then you’ll spend time profiling and stuff, so it’s a maintenance issue.”
If there’s a new issue type, what would its icon be? A snail? (I could never figure out the icon for code smell – is that a radiation symbol, or a reel of magtape, or what? I guess that’s more “professional” than something like U+1F4A9 )
I could see where there could be some adaptive severity ratings involved. Whereas most issue rules have one severity setting (either default or in the quality profile), the severity of a perf issue could depend on how it’s used. For instance, there’s the rule java:S1157 about converting the case of strings just to do a case-insensitive compare, a minor smell. That currently does not have a perf tag, but if that gets made a perf issue, it would probably be minor still. However, if SQ notices it’s in a loop, maybe it could elevate the severity to major, since you’re multiplying the perf impact by the loop count.
Anyway, if there’s a new category, I hope it is also an issue category and doesn’t go and introduce a new set of APIs that 1) break existing scripts, and 2) have far less capabilities.
But then again, some people like garlic, so you might be encouraging them to add more code smells. Whereas nobody likes U+1F4A9, except for putting on T-shirts!
)
Whereas nobody likes U+1F4A9, except for putting on T-shirts!