-
Notifications
You must be signed in to change notification settings - Fork 889
Add rule: strict-string-expressions #4807
Add rule: strict-string-expressions #4807
Conversation
@ColCh looks like you've got some lint failures from CI running this rule on TSLint itself. https://circleci.com/gh/palantir/tslint/16921?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link That looks like it's exposed an edge case or two. Should function complain(error?: Error | null) {
console.error(`Oh dear: ${error}`);
}
If you really wanted, you could make an issue to add an option to disallow this, or file a followup issue after this PR... but IMO it's fine to just always allow these. |
Yes, I agree with you. That I think that better option is make those as configurable thing |
@@ -187,7 +187,7 @@ export class Linter { | |||
this.options.formatter !== undefined ? this.options.formatter : "prose"; | |||
const Formatter = findFormatter(formatterName, this.options.formattersDirectory); | |||
if (Formatter === undefined) { | |||
throw new Error(`formatter '${formatterName}' not found`); | |||
throw new Error(`formatter '${String(formatterName)}' not found`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JoshuaKGoldberg seems to pass now I don't understand why so... please review :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for pushing this through @ColCh! (and the helpful PR comments!)
Thank you! |
PR checklist
???
Overview of change:
adds implementation for rule discussed in #4512
Is there anything you'd like reviewers to focus on?
CHANGELOG.md entry:
[new-rule]
strict-string-expressions
reports errors on type coercions found in string expressions