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

feat(eslint-plugin): add quotes [extension] #762

Merged
merged 9 commits into from
Aug 19, 2019

Conversation

a-tarasyuk
Copy link
Contributor

@a-tarasyuk a-tarasyuk commented Jul 26, 2019

Fixes #757
Fixes #756

@a-tarasyuk a-tarasyuk force-pushed the feature/quotes branch 3 times, most recently from 5789ece to f74f9d3 Compare July 26, 2019 11:07
@bradzacher bradzacher added the enhancement: new plugin rule New rule request for eslint-plugin label Jul 26, 2019
Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

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

few very minor nits, otherwise code looks good to me!

Could you please add the rule name to this Set here and regenerate the configs:
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/tools/generate-configs.ts#L21-L32

Thanks for doing this!

packages/eslint-plugin/src/rules/quotes.ts Outdated Show resolved Hide resolved
packages/eslint-plugin/src/rules/quotes.ts Outdated Show resolved Hide resolved
packages/eslint-plugin/src/rules/quotes.ts Outdated Show resolved Hide resolved
packages/eslint-plugin/src/rules/quotes.ts Outdated Show resolved Hide resolved
@bradzacher bradzacher added enhancement: new base rule extension New base rule extension required to handle a TS specific case and removed enhancement: new plugin rule New rule request for eslint-plugin labels Jul 26, 2019
@a-tarasyuk
Copy link
Contributor Author

@bradzacher added changes.

Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

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

LGTM - thank you

@bradzacher bradzacher added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Jul 29, 2019
@LekoArts
Copy link

LekoArts commented Jul 31, 2019

Would be great to have that merged and released! Just stumbled upon the initial issue

Copy link
Member

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

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

Thanks a lot @a-tarasyuk!

@JamesHenry JamesHenry removed the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Aug 19, 2019
@codecov
Copy link

codecov bot commented Aug 19, 2019

Codecov Report

Merging #762 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #762      +/-   ##
==========================================
+ Coverage   94.13%   94.15%   +0.02%     
==========================================
  Files         112      113       +1     
  Lines        4843     4860      +17     
  Branches     1347     1351       +4     
==========================================
+ Hits         4559     4576      +17     
  Misses        163      163              
  Partials      121      121
Impacted Files Coverage Δ
packages/eslint-plugin/src/rules/quotes.ts 100% <100%> (ø)
packages/eslint-plugin/src/rules/index.ts 100% <100%> (ø) ⬆️

@JamesHenry JamesHenry merged commit 9f82099 into typescript-eslint:master Aug 19, 2019
LekoArts added a commit to LekoArts/gatsby-themes that referenced this pull request Sep 11, 2019
Since typescript-eslint/typescript-eslint#762 is published the normal quotes rule can be disabled and the typescript one activated. This allows me to use backticks, but they won’t get added to types (they are not valid there)
@regevbr
Copy link
Contributor

regevbr commented Mar 18, 2020

Hi @JamesHenry @a-tarasyuk
Thanks for the fix, but there is 1 more case which is not covered here.
When you need to import from old modules that use the following syntax

export = ...

You must use

import moment = require('moment'); // real life example

to import them, and the rule alerts on those, while it is not valid to use back ticks here.

Do you can care to fix that as well?

@bradzacher
Copy link
Member

hey @regevbr - please raise a new issue instead of commenting on old PRs.

@typescript-eslint typescript-eslint locked as resolved and limited conversation to collaborators Mar 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement: new base rule extension New base rule extension required to handle a TS specific case
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[quotes] False positive with backtick in declare module [quotes] False positive requiring backticks in types
5 participants