Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Rule suggestion: restrict + operands #697

Closed
myitcv opened this issue Sep 30, 2015 · 13 comments
Closed

Rule suggestion: restrict + operands #697

myitcv opened this issue Sep 30, 2015 · 13 comments

Comments

@myitcv
Copy link
Contributor

myitcv commented Sep 30, 2015

With reference to the binary + operator in the spec

I find it rather disconcerting that this is allowed:

let x = 5 + "test";
let y = true + "test";

Would there be interest in a rule that forces the two operands to + to either:

  • both be of type string
  • both be of type number

and fail otherwise?

Do we have the necessary type information available during linting to do this?

@myitcv
Copy link
Contributor Author

myitcv commented Sep 30, 2015

I've made a start, but just run into a problem

@adidahiya
Copy link
Contributor

sounds like a good lint rule, thanks for looking into this!

@adidahiya
Copy link
Contributor

TSLint currently only gets type information on a per-file basis, so to do this properly, I think we're blocked on #680

@myitcv
Copy link
Contributor Author

myitcv commented Sep 30, 2015

Ah. Is that why the TypeChecker returns Any?

@myitcv
Copy link
Contributor Author

myitcv commented Sep 30, 2015

Sorry, to be more explicit: that question relates to the TypeScript issue I created

@adidahiya
Copy link
Contributor

No, I'm not sure why the type checker returns any in that issue you created -- intuitively it feels like the checker has enough information about the built-in .toString() to report the correct return type.

Perhaps we're not instantiating the checker correctly?

@myitcv
Copy link
Contributor Author

myitcv commented Sep 30, 2015

Hmm, having done a quick test I think it is related. In the following example the TypeChecker (in the way we are using it with a single file) reports the type of Banana() is Any:

import {Banana} from "./somewhere_else";

// Banana has type signature: Banana(): string

let y = "test" + Banana();

My guess would be that when it hits an identifier defined outside of the scope of what is being checked it assumes an Any type

@jkillian
Copy link
Contributor

Right, there's obviously no way for the TypeChecker to know what Banana() is if it's only given that one file. Maybe the same thing applies for the built-in definitions: possibly since lib.core.d.ts isn't included in the program it has no awareness of core typings.

@myitcv
Copy link
Contributor Author

myitcv commented Sep 30, 2015

@jkillian exactly. So #680 will definitely allow us to write such rules. Look forward to it landing!

@alexeagle
Copy link
Contributor

alexeagle commented Jun 12, 2016

This one is approved to be implemented in tsc now. microsoft/TypeScript#7989 (comment)

@myitcv
Copy link
Contributor Author

myitcv commented Jun 13, 2016

@alexeagle but only if you use --strictNullChecks

@adidahiya
Copy link
Contributor

@alexeagle thanks for the link. Agreed with @myitcv that it can still be a TSLint rule that is a more granular opt-in than --strictNullChecks in the compiler.

ScottSWu added a commit to ScottSWu/tslint that referenced this issue Jun 16, 2016
* Changed Linter to accept multiple files (constructor overload)
* Created TypedRule class which accepts a Program with a SourceFile
* Rules extending this class can now access the type checker by
  implementing applyWithProgram
* Added an example restrictPlusOperands rule (palantir#697) that uses the types
* Added a fixes interface to RuleFailure, allowing text-replacement
  suggested fixes
ScottSWu added a commit to ScottSWu/tslint that referenced this issue Jun 17, 2016
* Changed Linter to accept multiple files (constructor overload)
* Created TypedRule class which receives a Program with a SourceFile
* Rules extending this class can now access the type checker by
  implementing applyWithProgram
* Added an example restrictPlusOperands rule (palantir#697) that uses types
ScottSWu added a commit to ScottSWu/tslint that referenced this issue Jun 24, 2016
* Changed Linter to accept multiple files (constructor overload)
* Created TypedRule class which receives a Program with a SourceFile
* Rules extending this class can now access the type checker by
  implementing applyWithProgram
* Added an example restrictPlusOperands rule (palantir#697) that uses types
ScottSWu added a commit to ScottSWu/tslint that referenced this issue Jun 27, 2016
* Changed Linter to accept multiple files (constructor overload)
* Created TypedRule class which receives a Program with a SourceFile
* Rules extending this class can now access the type checker by
  implementing applyWithProgram
* Added an example restrictPlusOperands rule (palantir#697) that uses types
ScottSWu added a commit to ScottSWu/tslint that referenced this issue Jun 30, 2016
This rule restricts the + operands to either both numbers or both
strings.

Relates to palantir#697
ScottSWu added a commit to ScottSWu/tslint that referenced this issue Jun 30, 2016
This rule restricts the + operands to either both numbers or both
string.

Relates to palantir#697
ScottSWu added a commit to ScottSWu/tslint that referenced this issue Jul 1, 2016
This rule restricts the + operands to either both numbers or both
string.

Relates to palantir#697
@OliverJAsh
Copy link
Contributor

Can this be closed now we have https://palantir.github.io/tslint/rules/restrict-plus-operands/?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants