16f6c0aecf
Inline adoc files when they are included exactly once. Also fix language tags because this inlining gives us better information on what language the code is written in.
130 lines
3.7 KiB
Plaintext
130 lines
3.7 KiB
Plaintext
== Why is this an issue?
|
|
|
|
The use of comparison operators (``++<++``, ``++<=++``, ``++>=++``, ``++>++``) with strings is not likely to yield the expected results. Make sure the intention was to compare strings and not numbers.
|
|
|
|
|
|
=== Noncompliant code example
|
|
|
|
[source,javascript]
|
|
----
|
|
var appleNumber = "123";
|
|
var orangeNumber = "45";
|
|
if (appleNumber < orangeNumber) { // Noncompliant, this condition is true
|
|
alert("There are more oranges");
|
|
}
|
|
----
|
|
|
|
|
|
=== Compliant solution
|
|
|
|
[source,javascript]
|
|
----
|
|
var appleNumber = "123";
|
|
var orangeNumber = "45";
|
|
if (Number(appleNumber) < Number(orangeNumber)) {
|
|
alert("There are more oranges");
|
|
}
|
|
----
|
|
|
|
|
|
=== Exceptions
|
|
|
|
The rule ignores string comparisons occurring in the callback of a sort invocation, e.g.:
|
|
|
|
[source,javascript]
|
|
----
|
|
const fruits = ['orange', 'apple', 'banana'];
|
|
fruits.sort((a, b) => a < b);
|
|
----
|
|
|
|
|
|
ifdef::env-github,rspecator-view[]
|
|
|
|
'''
|
|
== Implementation Specification
|
|
(visible only on this page)
|
|
|
|
=== Message
|
|
|
|
Convert operands of this use of "[<|>|+<=+|>=]" to number type.
|
|
|
|
|
|
=== Highlighting
|
|
|
|
* Primary: comparison operator
|
|
* Secondary: both string operands
|
|
|
|
|
|
'''
|
|
== Comments And Links
|
|
(visible only on this page)
|
|
|
|
=== on 18 Nov 2015, 17:47:57 Elena Vilchik wrote:
|
|
\[~ann.campbell.2] I'm stuck with this rule. Could you help me with a draft?
|
|
|
|
Initially idea was to check that binary operations are called with operands of same type. But further investigations shown that it's common practice to use what ever types you want :)
|
|
|
|
So I found out cases when expected result might differ from actual (see SONARJS-450), and I think that this rule should check only these particular cases. Now I don't know what should be the description and especially the title of this rule.
|
|
|
|
=== on 18 Nov 2015, 19:51:22 Ann Campbell wrote:
|
|
Okay [~elena.vilchik], let's narrow this down. We want to raise an issue when:
|
|
|
|
|
|
* String is compared (<, >, +<=+, >=, ==(?), ===(?)) with ... anything? Linked ticket shows ``++str1 < str2++`` but it seems that it would be equally faulty with ``++str1 < obj++`` and ``++str1 >= 4++`` ...?
|
|
* Strings are concatenated with non-strings
|
|
* dis-similar types are checked for (in)equality. Is this only ``++===++`` or ``++==++`` as well?
|
|
|
|
=== on 19 Nov 2015, 11:34:42 Elena Vilchik wrote:
|
|
\[~ann.campbell.2] Not exactly. It should be this way:
|
|
|
|
* string compared with string only (<, >, +<=+, >=)
|
|
|
|
----
|
|
"123" < "14" // true, lexical comparison <--- Noncompliant
|
|
"123" < 14 // false, both casted to numbers
|
|
----
|
|
Comparison of string with other types is not so dangerous (because actual behaviour is quite expected).
|
|
|
|
* string concatenated with numbers (because some could expect addition)
|
|
|
|
----
|
|
"123" + 45 // "12345" <--- Noncompliant
|
|
----
|
|
|
|
* check only "===" for any non identical types
|
|
|
|
----
|
|
"1" == 1 // true
|
|
"1" === 1 // false, whatever values, if types are different <--- Noncompliant
|
|
----
|
|
|
|
=== on 19 Nov 2015, 19:53:49 Ann Campbell wrote:
|
|
What do you think of splitting this into two rules:
|
|
|
|
* Faulty string operations should not be made
|
|
* "===" should not be used with dissimilar types
|
|
?
|
|
|
|
|
|
Otherwise, the best I've got is: Faulty operations should not be made
|
|
|
|
=== on 20 Nov 2015, 09:46:42 Elena Vilchik wrote:
|
|
\[~ann.campbell.2] We decided to split it to 3 rules :)
|
|
|
|
* String comparisons should not be made (major with suspicious tag)
|
|
* Numbers should be added to strings (major with suspicious tag)
|
|
* "===" should not be used with dissimilar types (critical with bug tag)
|
|
(welcome to change these titles)
|
|
|
|
Could you create RSPECs?
|
|
|
|
|
|
|
|
=== on 20 Nov 2015, 16:38:40 Elena Vilchik wrote:
|
|
\[~ann.campbell.2] Looks like i explained rule badly. I changed description so that now IMO it reflect the rule idea. Could you check it?
|
|
|
|
=== on 20 Nov 2015, 16:48:39 Ann Campbell wrote:
|
|
Looks good [~elena.vilchik]
|
|
|
|
endif::env-github,rspecator-view[]
|