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

Fix quotemark avoid-template issues #4408

Merged
merged 4 commits into from
Jan 10, 2019
Merged

Fix quotemark avoid-template issues #4408

merged 4 commits into from
Jan 10, 2019

Conversation

johnwiseheart
Copy link
Contributor

@johnwiseheart johnwiseheart commented Dec 22, 2018

PR checklist

Overview of change:

Add an additonal check to make sure we never render a warning of the format " should be " or ` should be `.

Is there anything you'd like reviewers to focus on?

Whats the best test for this? I couldn't find a way to write a test that failed before making this fix, but I feel like we shouldn't merge this without one.

CHANGELOG.md entry:

[bugfix] fix some quotemark issues when using backticks with the avoid-template option

Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Also waiting on a test

src/rules/quotemarkRule.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

I can't repro the original bug, and the lack of test case seems to suggest that it's not really a problem?

@adidahiya adidahiya self-assigned this Jan 10, 2019
@adidahiya
Copy link
Contributor

the fix in the first commit doesn't work; I added some better test cases. there are issues around the rule behavior when both avoid-escape and avoid-template are enabled. I'll fix this up

@@ -27,12 +27,12 @@ const OPTION_JSX_DOUBLE = "jsx-double";
const OPTION_AVOID_TEMPLATE = "avoid-template";
const OPTION_AVOID_ESCAPE = "avoid-escape";

type QUOTE_MARK = "'" | '"' | "`";
type JSX_QUOTE_MARK = "'" | '"';
type QUOTEMARK = "'" | '"' | "`";
Copy link
Contributor

Choose a reason for hiding this comment

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

I renamed "quote mark" to "quotemark" everywhere to be consistent

);
}
ts.forEachChild(node, cb);
});
}

function getQuotemarkPreference(args: any[]): QUOTE_MARK {
type QUOTE_PREF = typeof OPTION_SINGLE | typeof OPTION_DOUBLE | typeof OPTION_BACKTICK;
Copy link
Contributor

Choose a reason for hiding this comment

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

also simplified these two helper functions to reduce duplication and the extra typedefs

@adidahiya adidahiya changed the title Fix quotemark rule erroneous warning Fix quotemark avoid-template issues Jan 10, 2019
@adidahiya
Copy link
Contributor

thanks @JoshuaKGoldberg

@adidahiya adidahiya merged commit 9cd8af3 into master Jan 10, 2019
@adidahiya adidahiya deleted the jw/fix-quotemark branch January 10, 2019 21:53
adidahiya pushed a commit that referenced this pull request Jan 10, 2019
ColCh added a commit to ColCh/tslint that referenced this pull request Feb 3, 2019
* master: (60 commits)
  Added tslint-brunch to the list of 3rd party tools (palantir#4251)
  Switch to tslint-plugin-prettier, clean up rule options config syntax (palantir#4488)
  Enable grouped-imports for ordered-imports rule in tslint:all config (palantir#4420)
  Ordered imports grouping (palantir#4134)
  trailing-comma: check for a closing parenthesis (palantir#4457)
  Update index.md (palantir#4473)
  [bugfix] `no-unsafe-any`: allow implicitly downcasting `any` to `unknown` (palantir#4442)
  Add v5.12.1 changelog
  Bump version to 5.12.1
  Fix quotemark avoid-template issues (palantir#4408)
  Skip linting JSON files entirely (palantir#4001)
  Fix strict-type-predicate for unknown (palantir#4444)
  restrict increment-decrement fixer while fixing the postfix unary expressions (palantir#4415)
  Mention file names in script parse failures (palantir#4397)
  Revert breaking change to tslint:recommended, update tslint:latest (palantir#4404)
  Fix quotemark avoid-template issues (palantir#4408)
  Bump tslint dev dependency to 5.12.0 (palantir#4452)
  Skip linting JSON files entirely (palantir#4001)
  Fix strict-type-predicate for unknown (palantir#4444)
  [README] Update link for Webstorm (palantir#4450)
  ...
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.

3 participants