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): added new rule typedef #581

Merged
merged 11 commits into from Jul 24, 2019

Conversation

JoshuaKGoldberg
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg commented Jun 1, 2019

Adds the equivalent of TSLint's typedef rule.

Closes #558.

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.

A few questions for you.
Had a quick eyeball of the rule code and it looks okay. I'll have a deeper look after these questions

packages/eslint-plugin/docs/rules/typedef.md Outdated Show resolved Hide resolved
packages/eslint-plugin/docs/rules/typedef.md Show resolved Hide resolved
packages/eslint-plugin/docs/rules/typedef.md Outdated Show resolved Hide resolved
packages/eslint-plugin/docs/rules/typedef.md Show resolved Hide resolved
packages/eslint-plugin/docs/rules/typedef.md Show resolved Hide resolved
packages/eslint-plugin/docs/rules/typedef.md Outdated Show resolved Hide resolved
packages/eslint-plugin/docs/rules/typedef.md Outdated Show resolved Hide resolved
packages/eslint-plugin/docs/rules/typedef.md Show resolved Hide resolved
packages/eslint-plugin/src/rules/typedef.ts Outdated Show resolved Hide resolved
@Shinigami92
Copy link

Shinigami92 commented Jun 19, 2019

I really love the TSLint typedef rule and I cant migrate to eslint without the support of this rule. This said, I have one proposal what can improve typedef. Let us decide which option throws a warning and which throws an error -> so with this CI/CD can fail only on some of the options.

@bradzacher
Copy link
Member

@Shinigami92 - this is not possible - eslint only allows for a single error level for a rule. Either it's all a warning, or it's all an error.

@Shinigami92
Copy link

Ok :/ sad but ok

@Shinigami92
Copy link

TSLint currently introduces a new option variable-declaration-ignore-function
palantir/tslint#2654

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.

as mentioned, remove arrowCallSignature and callSignature from the rule.
document that users should instead use explicit-function-return-type.

Correct me if I'm wrong here - but looking at the options, I don't think that it needs to access parserServices at all - it's just checking for missing AST nodes. So if I'm not wrong, please remove the dependency on the parser services.

@bradzacher bradzacher added enhancement: new plugin rule New rule request for eslint-plugin and removed enhancement New feature or request labels Jun 28, 2019
@JamesHenry JamesHenry added the awaiting response Issues waiting for a reply from the OP or another party label Jul 12, 2019
@bradzacher bradzacher added 1 approval PR that a maintainer has LGTM'd - any maintainer can merge this when ready and removed awaiting response Issues waiting for a reply from the OP or another party labels Jul 17, 2019
@JamesHenry JamesHenry removed the 1 approval PR that a maintainer has LGTM'd - any maintainer can merge this when ready label Jul 24, 2019
@codecov
Copy link

codecov bot commented Jul 24, 2019

Codecov Report

Merging #581 into master will increase coverage by 0.01%.
The diff coverage is 96.66%.

@@            Coverage Diff             @@
##           master     #581      +/-   ##
==========================================
+ Coverage   94.45%   94.47%   +0.01%     
==========================================
  Files         111      112       +1     
  Lines        4690     4720      +30     
  Branches     1291     1306      +15     
==========================================
+ Hits         4430     4459      +29     
  Misses        150      150              
- Partials      110      111       +1
Impacted Files Coverage Δ
packages/eslint-plugin/src/rules/index.ts 100% <100%> (ø) ⬆️
packages/eslint-plugin/src/rules/typedef.ts 96.55% <96.55%> (ø)

@JamesHenry JamesHenry merged commit 35cc99b into typescript-eslint:master Jul 24, 2019
@JoshuaKGoldberg JoshuaKGoldberg deleted the typedef branch July 25, 2019 00:32
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement: new plugin rule New rule request for eslint-plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[typedef] What is ETA of the typedef rule?
4 participants