Skip to content

Update tslint.json for latest versions of tslint #3995

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

Merged
merged 1 commit into from
Aug 4, 2015

Conversation

weswigham
Copy link
Member

Enforces spaces and double quotes again, and enforces no space before the colon for type definitions.

This should fix part of #3994. I couldn't find an existing rule to cover the no multiple declaration expressions style, so that is missing from this.

Enforces spaces and double quotes again, and enforces no space before the colon for type definitions.
@DanielRosenwasser
Copy link
Member

Looks great, thanks @weswigham! @danquirk can you take a quick look?

@danquirk
Copy link
Member

I think the rule changes are all good but I'm inclined to say we shouldn't modify the rules without an accompanying update to the affected code. Ideally the only TSLint violations are things that TSLint doesn't support yet (ex namespaces) plus a very small number of non-standard patterns we might need to evaluate. I was hoping that the former would be taken care of soon (I need to check on TSLint's 1.5 support) to a degree where we could consider turning on the linting by default.

@weswigham
Copy link
Member Author

The jake lint task is broken right now. I actually haven't noticed since I've been calling tslint on my files directly.

I think this line needs to be changed to

    var cmd = 'tslint -c tslint.json ' + f;

I'll make a PR for it.

@danquirk
Copy link
Member

How is it broken? Seems to be working fine on my machine:

F:\GitHub\TypeScript [master +5 ~0 -0 !]> jake lint
src/compiler/sys.ts[300, 22]: unreachable code

...<elided>

src/compiler/sys.ts[34, 6]: missing semicolon
FAILURE: Please fix linting errors in src\compiler\sys.ts

SUCCESS: No linter errors in src\harness\harnessLanguageService.ts
<and so on>

@weswigham
Copy link
Member Author

usage: node F:\ts\typescript\node_modules\tslint\bin\tslint

Options:
  -c, --config          configuration file
  -h, --help            display detailed help
  -o, --out             output file
  -r, --rules-dir       rules directory
  -s, --formatters-dir  formatters directory
  -t, --format          output format (prose, json, verbose)  [default: "prose"]
  -v, --version         current version

Missing files
FAILURE: Please fix linting errors in src\harness\rwcRunner.ts

etc.

There's no -f in the latest tslint, it just expects the files to be passed last.

@danquirk
Copy link
Member

Oh I see, they took a breaking change in their cmd line args.

@weswigham
Copy link
Member Author

We've merged in #4005, to fix the issue in the conversation above. Now this needs to be merged to enable linting on tabs, quotes, and spaces before colons.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 1, 2015

👍

@mhegazy
Copy link
Contributor

mhegazy commented Aug 1, 2015

Are there code changes needed from these changes?

@weswigham
Copy link
Member Author

The tslint isn't actually used in a build (other than jake lint, which wasn't even in the README until recently) yet since the codebase doesn't yet pass the lint, so merging changes to it should be harmless.

#4008 fixes all the linter errors this tslint reports that aren't apparently caused by bugs in tslint (or, more accurately, by tslint not supporting TS 1.6 features yet), afaik.

mhegazy added a commit that referenced this pull request Aug 4, 2015
Update tslint.json for latest versions of tslint
@mhegazy mhegazy merged commit bb3fb7d into microsoft:master Aug 4, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Aug 4, 2015

thanks

@weswigham weswigham deleted the patch-2 branch August 17, 2017 23:02
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
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.

5 participants