rspec/rules/S3003/javascript/rule.adoc

== 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[]