Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Add disallowVar lint rule #107

Merged
merged 1 commit into from
Mar 3, 2020
Merged

Add disallowVar lint rule #107

merged 1 commit into from
Mar 3, 2020

Conversation

jamiebuilds
Copy link
Contributor

@jamiebuilds jamiebuilds commented Mar 2, 2020

Just going through the list here: #20 (comment) Wanted to grab something easy.

import {Path} from '@romejs/js-compiler';

const DEFAULT_BASENAME_MESSAGE =
'Variable declarations using `var` are disallowed, use `let` or `const` instead.';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possible improvement: Automatically suggest let or const based on how the variable is currently used (or at least refine down to just let if it gets reassigned). If the variable isn't used yet (which will happen often when linting while typing) it could use the default message.

@sebmck
Copy link
Contributor

sebmck commented Mar 2, 2020

Missing a test, otherwise looks good.

@sebmck sebmck mentioned this pull request Mar 2, 2020
27 tasks
@jamiebuilds jamiebuilds force-pushed the disallowVar branch 3 times, most recently from 576aebf to b1960dd Compare March 2, 2020 21:52
var littleOffset = 26;
var numberOffset = 52;
const littleOffset = 26;
const numberOffset = 52;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding this lint rule caught some stuff

Copy link
Contributor

Choose a reason for hiding this comment

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

dope

@sebmck sebmck merged commit 720e1dc into rome:master Mar 3, 2020
@sebmck
Copy link
Contributor

sebmck commented Mar 3, 2020

Thank you!

@jamiebuilds jamiebuilds deleted the disallowVar branch March 3, 2020 17:54
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