feat(typescript-estree)!: throw error on file not in project when project set

[uniqueiniquity]

BREAKING CHANGE

Currently, when using @typescript-eslint/parser with the project option, there is no connection between the set of files as defined by the tsconfig.json file(s) you provide and the files that you're actually linting. It turns out that if you lint a file not in that set, the process of building enough context to drive semantic rules incurs a significant performance hit.

Here, we introduce an error when such a file is linted when the project setting is provided. This will help users avoid incurring this performance hit, while explicitly alerting them of what issue is occurring (rather than simply not running semantic lint rules on such files). Note that this error matches TSLint's behavior in this scenario.

Fixes #724

[codecov]

Codecov Report

Merging #760 into master will decrease coverage by 0.05%.
The diff coverage is 92.3%.

@@            Coverage Diff            @@
##           master    #760      +/-   ##
=========================================
- Coverage   94.26%   94.2%   -0.06%     
=========================================
  Files         112     112              
  Lines        4793    4801       +8     
  Branches     1333    1334       +1     
=========================================
+ Hits         4518    4523       +5     
- Misses        158     159       +1     
- Partials      117     119       +2

Impacted Files	Coverage Δ
packages/typescript-estree/src/tsconfig-parser.ts	87.83% <100%> (-0.74%)	⬇️
packages/typescript-estree/src/parser.ts	93.54% <88.88%> (-1.46%)	⬇️

[bradzacher]

thanks for doing this!

we probably need to test this with a vue project, just so we can see how this interacts with vue-eslint-parser.

code itself looks good for me.

[uniqueiniquity]

@bradzacher I've addressed your comments and synced with master.
The tests for no-unnecessary-assertion are totally busted on my machine but passing fine here... with or without my most recent commits. I'll probably delete the whole folder and try again locally.

I haven't had a chance to try with vue yet, but I will as soon as I can (tomorrow or this weekend).
I'm not concerned, though, as long as the vue parser is giving us typescript code to parse with the .vue filename, and as long as projects are set up as the vetur documentation suggests.

[uniqueiniquity]

Actually, just tried it with a project generated by the Vue CLI and hit an issue.
Will investigate more when less tired :)

Edit: Couldn't help myself from looking more.
Seems fine to me.
Here's what I did:

• Using the Vue CLI, vue create hello-world with babel, eslint, and typescript
• set the parserOptions in the generated .eslintrc.js to:

  parserOptions: {
    parser: '@typescript-eslint/parser',
    project: `${__dirname}/tsconfig.json`,
    extraFileExtensions: [".vue"]
  }

• Run ESLint on App.vue

My mistake earlier was forgetting the extraFileExtensions property. Without that, we get the new error. With it, it lints properly. It even correctly parses the components/HelloWorld.vue file when creating the project for linting... I was surprised that that worked, but I'm happy that it did. :)

[bradzacher]

Just to confirm - did you try that on a SFC (Single File Component)? (eg below)

Based on #404 I wasn't sure if it'd work.
If so then that's great! We can mark that as closed.

Just for sanity can you add a test like this, just so we can make sure that we don't break support again.
(I've never used vue, but I'm pretty sure that this is valid vue code):

// .eslint.js
module.exports = {
  parser: 'vue-eslint-parser',
  parserOptions: {
    "parser": '@typescript-eslint/parser',
    "project": `${__dirname}/tsconfig.json`,
    extraFileExtensions: ['.vue'],
  },
  plugins: ['@typescript-eslint'],
  rules: {
    '@typescript-eslint/no-implicit-any': 'error',
  },
};

<!-- Hello.vue -->
<template>
    <div>
        <div>
		    <!-- !!!!! expected error !!!!! -->
			Hello {{ (name as any) }}{{exclamationMarks}}
		</div>
        <button @click="decrement">-</button>
        <button @click="increment">+</button>
    </div>
</template>

<script lang="ts">
import Vue from "vue";
export default Vue.extend({
    props: ['name', 'initialEnthusiasm'],
    data() {
        return {
            enthusiasm: this.initialEnthusiasm,
        }
    },
    methods: {
        increment() { this.enthusiasm++; },
        decrement() {
            if (this.enthusiasm > 1) {
                this.enthusiasm--;
            }
        },
    },
    computed: {
        exclamationMarks(): any { // !!!!! expected error !!!!!
            return Array(this.enthusiasm + 1).join('!');
        }
    }
});
</script>

[uniqueiniquity]

Yep, I can lint that file fine (assuming you meant no-explicit-any).
One thing to point out is that only the any in the script tag is flagged.
In order to traverse the template, the rule needs to use the context.parserServices.defineTemplateBodyVisitor, and since ours isn't vue-specific, we don't traverse the expression containing the other any.

Is the best way to add that test via another integration test with docker?
I'm happy to do it, but I don't really know anything about docker so it might take me a bit :)

[bradzacher]

Awesome, it's good that it works!

Is the best way to add that test via another integration test with docker?

That's a good question for @JamesHenry - I don't know that test harness at all.

---

only the any in the script tag is flagged
context.parserServices.defineTemplateBodyVisitor

I didn't know about that function ...
Do we want to add defineTemplateBodyVisitor (in a separate PR)?

[JamesHenry]

I'll add the integration test in a follow up, thanks again @uniqueiniquity!