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

Fix restrict-plus-operands for ternary ops #1925

Merged
merged 2 commits into from Dec 30, 2016
Merged

Fix restrict-plus-operands for ternary ops #1925

merged 2 commits into from Dec 30, 2016

Conversation

nchen63
Copy link
Contributor

@nchen63 nchen63 commented Dec 23, 2016

fixes #1922

@adidahiya adidahiya changed the title Fix restruct-plus-operands for ternary ops Fix restrict-plus-operands for ternary ops Dec 24, 2016
@@ -40,6 +41,8 @@ export class Rule extends Lint.Rules.TypedRule {
}
}

type AddTypes = "string" | "number" | "invalid";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need a new type for this; I would prefer the union inlined in two places. This change makes the code more confusing

@@ -60,12 +63,34 @@ class RestrictPlusOperandsWalker extends Lint.ProgramAwareRuleWalker {
}
}

function getAddableType(node: ts.Expression, tc: ts.TypeChecker): "string" | "number" | "invalid" {
function getAddableType(node: ts.Expression, tc: ts.TypeChecker): AddTypes {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this extra function in the call stack is necessary -- would prefer to inline usage of getBaseTypeOfLiteralType above, like so:

const tc = this.getTypeChecker();
const leftType = getBaseTypeOfLiteralType(ts.getTypeAtLocation(node.left));
...

}
return "invalid";
}

function allSame(array: string[]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be nicer with Array#reduce

@nchen63 nchen63 merged commit 3b718d6 into master Dec 30, 2016
@adidahiya adidahiya deleted the fix-plus branch January 3, 2017 17:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

restrict-plus-operands false positive with ternary operator
2 participants