Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[restrict-plus-operands] false positive on type parameters #230

Closed
mysticatea opened this issue Feb 8, 2019 · 1 comment · Fixed by #440
Closed

[restrict-plus-operands] false positive on type parameters #230

mysticatea opened this issue Feb 8, 2019 · 1 comment · Fixed by #440
Labels
bug Something isn't working enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@mysticatea
Copy link
Member

Repro

{
  "plugins": ["@typescript-eslint"],
  "rules": {
    "@typescript-eslint/restrict-plus-operands": "error"
  }
}
function foo<T extends string>(a: T) {
    return a + "" // error
}
function foo<T extends "a" | "b">(a: T) {
    return a + "" // error
}

Expected Result

No errors.

Actual Result

There are two errors at a + "".

  restrict-plus-operands
    valid
      √ var x = 5; (1469ms)
      √ var y = "10"; (736ms)
      √ var z = 8.2; (644ms)
      √ var w = "6.5"; (503ms)
      √ var foo = 5 + 10; (629ms)
      √ var foo = "5.5" + "10"; (490ms)
      √ var foo = parseInt("5.5", 10) + 10; (530ms)
      √ var foo = parseFloat("5.5", 10) + 10; (545ms)
      √ 
function test () : number { return 2; }
var foo = test("5.5", 10) + 10;
             (487ms)
      √ 
var x = 5;
var z = 8.2;
var foo = x + z;
             (646ms)
      √ 
var w = "6.5";
var y = "10";
var foo = y + w;
             (631ms)
      √ var foo = 1 + 1; (539ms)
      √ var foo = '1' + '1'; (509ms)
      √ 
var pair: { first: number, second: string } = { first: 5, second: "10" };
var foo = pair.first + 10;
             (558ms)
      √ 
var pair: { first: number, second: string } = { first: 5, second: "10" };
var foo = pair.first + (10 as number);
             (540ms)
      √ 
var pair: { first: number, second: string } = { first: 5, second: "10" };
var foo = "5.5" + pair.second;
             (513ms)
      √ 
var pair: { first: number, second: string } = { first: 5, second: "10" };
var foo = ("5.5" as string) + pair.second;
             (637ms)
      √ const foo = 'hello' + (someBoolean ? 'a' : 'b') + (() => someBoolean ? 'c' : 'd')() + 'e'; (606ms)
      √ const balls = true; (681ms)
      √ balls === true; (579ms)
      1) 
      function foo<T extends "a" | "b">(a: T) {
          return a + ""
      }
    
      2) 
      function foo<T extends string>(a: T) {
          return a + ""
      }
    
      √ 
      function foo(a: "a" | "b") {
          return a + ""
      }
     (566ms)
    invalid
      √ var foo = '1' + 1; (623ms)
      √ var foo = [] + {}; (554ms)
      √ var foo = 5 + "10"; (479ms)
      √ var foo = [] + 5; (495ms)
      √ var foo = [] + {}; (509ms)
      √ var foo = [] + []; (447ms)
      √ var foo = 5 + []; (592ms)
      √ var foo = "5" + {}; (544ms)
      √ var foo = 5.5 + "5"; (534ms)
      √ var foo = "5.5" + 5; (537ms)
      √ 
var x = 5;
var y = "10";
var foo = x + y;
             (642ms)
      √ 
var x = 5;
var y = "10";
var foo = y + x;
             (496ms)
      √ 
var x = 5;
var foo = x + {};
             (509ms)
      √ 
var y = "10";
var foo = [] + y;
             (515ms)
      √ 
var pair: { first: number, second: string } = { first: 5, second: "10" };
var foo = pair.first + "10";
             (483ms)
      √ 
var pair: { first: number, second: string } = { first: 5, second: "10" };
var foo = 5 + pair.second;
             (515ms)
      √ var foo = parseInt("5.5", 10) + "10"; (483ms)
      √ 
var pair = { first: 5, second: "10" };
var foo = pair + pair;
             (492ms)


  39 passing (24s)
  2 failing

  1) restrict-plus-operands
       valid

      function foo<T extends "a" | "b">(a: T) {
          return a + ""
      }
    :

      AssertionError [ERR_ASSERTION]: Should have no errors but had 1: [ { ruleId: 'restrict-plus-operands',
    severity: 1,
    message:
     "Operands of '+' operation must either be both strings or both numbers. Consider using a template literal.",
    line: 3,
    column: 18,
    nodeType: 'BinaryExpression',
    messageId: 'notStrings',
    endLine: 3,
    endColumn: 24 } ]
      + expected - actual

      -1
      +0

      at testValidTemplate (node_modules\eslint\lib\testers\rule-tester.js:415:20)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:584:25)

  2) restrict-plus-operands
       valid

      function foo<T extends string>(a: T) {
          return a + ""
      }
    :

      AssertionError [ERR_ASSERTION]: Should have no errors but had 1: [ { ruleId: 'restrict-plus-operands',
    severity: 1,
    message:
     "Operands of '+' operation must either be both strings or both numbers. Consider using a template literal.",
    line: 3,
    column: 18,
    nodeType: 'BinaryExpression',
    messageId: 'notStrings',
    endLine: 3,
    endColumn: 24 } ]
      + expected - actual

      -1
      +0

      at testValidTemplate (node_modules\eslint\lib\testers\rule-tester.js:415:20)
      at Context.RuleTester.it (node_modules\eslint\lib\testers\rule-tester.js:584:25)

Additional Info

Nothing in particular.

Versions

package version
@typescript-eslint/eslint-plugin master
@typescript-eslint/parser master
TypeScript 3.3.1
ESLint 5.12.1
node 11.9.0
npm 6.7.0
@mysticatea mysticatea added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Feb 8, 2019
@bradzacher
Copy link
Member

Technically speaking, this isn't a bug because it's not 100% typesafe to assume that a generic is the exact type that you've created a constraint on (i.e. a constraint doesn't mean "I am expecting the type is exactly a duck", you're saying "I expect that the type quacks like a duck").

However in this case because your generic constraint is asserting that it's a primitive type (i.e. can't be extended), I don't think there's a problem with supporting this case.
It would require extra type inspection.

@bradzacher bradzacher added bug Something isn't working and removed triage Waiting for team members to take a look labels Feb 8, 2019
@bradzacher bradzacher added the enhancement: plugin rule option New rule option for an existing eslint-plugin rule label Apr 19, 2019
@typescript-eslint typescript-eslint locked as resolved and limited conversation to collaborators Feb 21, 2020
Luckeu pushed a commit to Luckeu/typescript-eslint that referenced this issue Jun 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
2 participants