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

fix(typescript-estree): fix filename handling for vue JSX + markdown #1127

Merged
merged 12 commits into from Nov 3, 2019
Merged
@@ -1,7 +1,11 @@
import debug from 'debug';
import ts from 'typescript';
import { Extra } from '../parser-options';
import { ASTAndProgram, DEFAULT_COMPILER_OPTIONS } from './shared';
import {
ASTAndProgram,
DEFAULT_COMPILER_OPTIONS,
getScriptKind,
} from './shared';

const log = debug('typescript-eslint:typescript-estree:createIsolatedProgram');

Expand All @@ -10,7 +14,11 @@ const log = debug('typescript-eslint:typescript-estree:createIsolatedProgram');
* @returns Returns a new source file and program corresponding to the linted code
*/
function createIsolatedProgram(code: string, extra: Extra): ASTAndProgram {
log('Getting isolated program for: %s', extra.filePath);
log(
'Getting isolated program in %s mode for: %s',
extra.jsx ? 'TSX' : 'TS',
extra.filePath,
);

const compilerHost: ts.CompilerHost = {
fileExists() {
Expand All @@ -34,7 +42,13 @@ function createIsolatedProgram(code: string, extra: Extra): ASTAndProgram {
return '\n';
},
getSourceFile(filename: string) {
return ts.createSourceFile(filename, code, ts.ScriptTarget.Latest, true);
return ts.createSourceFile(
filename,
code,
ts.ScriptTarget.Latest,
true,
getScriptKind(extra, filename),
);
},
readFile() {
return undefined;
Expand Down
@@ -1,17 +1,23 @@
import debug from 'debug';
import ts from 'typescript';
import { Extra } from '../parser-options';
import { getScriptKind } from './shared';

const log = debug('typescript-eslint:typescript-estree:createIsolatedProgram');
const log = debug('typescript-eslint:typescript-estree:createSourceFile');

function createSourceFile(code: string, extra: Extra): ts.SourceFile {
log('Getting AST without type information for: %s', extra.filePath);
log(
'Getting AST without type information in %s mode for: %s',
extra.jsx ? 'TSX' : 'TS',
extra.filePath,
);

return ts.createSourceFile(
extra.filePath,
code,
ts.ScriptTarget.Latest,
/* setParentNodes */ true,
getScriptKind(extra),
);
}

Expand Down
30 changes: 30 additions & 0 deletions packages/typescript-estree/src/create-program/shared.ts
Expand Up @@ -38,11 +38,41 @@ function canonicalDirname(p: CanonicalPath): CanonicalPath {
return path.dirname(p) as CanonicalPath;
}

function getScriptKind(
extra: Extra,
filePath: string = extra.filePath,
): ts.ScriptKind {
const extension = path.extname(filePath);
// note - we respect the user's extension when it is known we could override it and force it to match their
// jsx setting, but that could create weird situations where we throw parse errors when TSC doesn't
switch (extension.toLowerCase()) {
case 'ts':
return ts.ScriptKind.TS;

case 'tsx':
return ts.ScriptKind.TSX;

case 'js':
return ts.ScriptKind.JS;
bradzacher marked this conversation as resolved.
Show resolved Hide resolved

case 'jsx':
return ts.ScriptKind.JSX;

case 'json':
return ts.ScriptKind.JSON;

default:
// unknown extension, force typescript to ignore the file extension, and respect the user's setting
return extra.jsx ? ts.ScriptKind.TSX : ts.ScriptKind.TS;
}
}

export {
ASTAndProgram,
canonicalDirname,
CanonicalPath,
DEFAULT_COMPILER_OPTIONS,
getCanonicalFileName,
getScriptKind,
getTsconfigPath,
};
18 changes: 18 additions & 0 deletions tests/integration/README.md
@@ -0,0 +1,18 @@
# Integration Tests

We have a set of integration tests defined in this project to help ensure we don't inadvertently break downstream packages that depend on us.

These tests are setup to run within docker containers to ensure that each test is completely isolated; we don't want them to affect our local environment, and similarly we don't want them to be effected by our local environment.

## Adding a new integration test

1. [Install docker for your platform](https://docs.docker.com/v17.09/engine/installation/#supported-platforms).
1. Add a new folder in `/tests/integration/fixtures`
1. Add a `.eslintrc.yml`, and a `tsconfig.json` to your folder, with the config required.
1. Create the necessary files to test the integration.
1. Copy+paste the `Dockerfile` from an existing fixture (they are all the same).
1. Copy+paste the `test.sh` from an existing fixture, and adjust the `eslint` command as required.
1. Add a new entry to `docker-compose.yml` by copy+pasting an existing section, and changing the name to match your new folder.
1. Add a new entry to `run-all-tests.sh` by copy+pasting an existing command, and changing the name to match your new folder.
1. Run your integration test by running the single command you copied in .
- If your test finishes successfully, a `test.js.snap` will be created.
40 changes: 37 additions & 3 deletions tests/integration/docker-compose.yml
Expand Up @@ -3,7 +3,7 @@ version: '3'
services:
typescript-and-tslint-plugins-together:
build: ./fixtures/typescript-and-tslint-plugins-together
container_name: "typescript-and-tslint-plugins-together"
container_name: 'typescript-and-tslint-plugins-together'
volumes:
# Runtime link to the relevant built @typescript-eslint packages and integration test utils,
# but apply an empty volume for the package tests, we don't need those.
Expand All @@ -22,7 +22,7 @@ services:

vue-sfc:
build: ./fixtures/vue-sfc
container_name: "vue-sfc"
container_name: 'vue-sfc'
volumes:
# Runtime link to the relevant built @typescript-eslint packages and integration test utils,
# but apply an empty volume for the package tests, we don't need those.
Expand All @@ -37,9 +37,26 @@ services:
# Runtime link to all the specific integration test files, so that most updates don't require a rebuild.
- ./fixtures/vue-sfc:/usr/linked

vue-jsx:
build: ./fixtures/vue-jsx
container_name: 'vue-jsx'
volumes:
# Runtime link to the relevant built @typescript-eslint packages and integration test utils,
# but apply an empty volume for the package tests, we don't need those.
- ../../package.json/:/usr/root-package.json
- ./utils/:/usr/utils
- ../../packages/parser/:/usr/parser
- /usr/parser/tests
- ../../packages/typescript-estree/:/usr/typescript-estree
- /usr/typescript-estree/tests
- ../../packages/eslint-plugin/:/usr/eslint-plugin
- /usr/eslint-plugin/tests
# Runtime link to all the specific integration test files, so that most updates don't require a rebuild.
- ./fixtures/vue-jsx:/usr/linked

recommended-does-not-require-program:
build: ./fixtures/recommended-does-not-require-program
container_name: "recommended-does-not-require-program"
container_name: 'recommended-does-not-require-program'
volumes:
# Runtime link to the relevant built @typescript-eslint packages and integration test utils,
# but apply an empty volume for the package tests, we don't need those.
Expand All @@ -53,3 +70,20 @@ services:
- /usr/eslint-plugin/tests
# Runtime link to all the specific integration test files, so that most updates don't require a rebuild.
- ./fixtures/recommended-does-not-require-program:/usr/linked

markdown:
build: ./fixtures/markdown
container_name: 'markdown'
volumes:
# Runtime link to the relevant built @typescript-eslint packages and integration test utils,
# but apply an empty volume for the package tests, we don't need those.
- ../../package.json/:/usr/root-package.json
- ./utils/:/usr/utils
- ../../packages/parser/:/usr/parser
- /usr/parser/tests
- ../../packages/typescript-estree/:/usr/typescript-estree
- /usr/typescript-estree/tests
- ../../packages/eslint-plugin/:/usr/eslint-plugin
- /usr/eslint-plugin/tests
# Runtime link to all the specific integration test files, so that most updates don't require a rebuild.
- ./fixtures/markdown:/usr/linked
23 changes: 23 additions & 0 deletions tests/integration/fixtures/markdown/.eslintrc.yml
@@ -0,0 +1,23 @@
root: true

# Local version of @typescript-eslint/parser
parser: '@typescript-eslint/parser'

env:
es6: true
node: true

parserOptions:
sourceType: module
extraFileExtensions: ['.vue']
ecmaFeatures:
jsx: true

plugins:
- 'markdown'
# Local version of @typescript-eslint/eslint-plugin
- '@typescript-eslint'

rules:
'@typescript-eslint/no-explicit-any': 'error'
'no-console': 'error'
71 changes: 71 additions & 0 deletions tests/integration/fixtures/markdown/Doc.md
@@ -0,0 +1,71 @@
Some extra text to verify that the markdown plugin is ignoring anything that is not a code block.

expected no-console error:
```jsx
import { Button } from 'antd';

function MyComp() {
console.log('test');
return (
<div>
<Button type="primary">Primary</Button>
<Button>Default</Button>
<Button type="dashed">Dashed</Button>
<Button type="danger">Danger</Button>
<Button type="link">Link</Button>
</div>
);
}
```

expected no-explicit-any error:
expected no-console error:
```jsx
import { Button } from 'antd';

function MyComp(): any {
console.log('test');
return (
<div>
<Button type="primary">Primary</Button>
<Button>Default</Button>
<Button type="dashed">Dashed</Button>
<Button type="danger">Danger</Button>
<Button type="link">Link</Button>
</div>
);
}
```

expected no-console error:
```js
function foo() {
console.log('test');
}
```

expected no-explicit-any error:
expected no-console error:
```js
function foo(): any {
console.log('test');
}
```


expected no-explicit-any error:
expected no-console error:
```javascript
function foo(): any {
console.log('test');
}
```


expected no-explicit-any error:
expected no-console error:
```node
function foo(): any {
console.log('test');
}
```
17 changes: 17 additions & 0 deletions tests/integration/fixtures/markdown/Dockerfile
@@ -0,0 +1,17 @@
FROM node:carbon

# Copy the test.sh into the container. Every other file will be linked, rather
# than copied to allow for changes without rebuilds wherever possible
WORKDIR /usr
COPY ./test.sh /usr/

# Create file which will be executed by jest
# to assert that the lint output is what we expect
RUN echo "const actualLintOutput = require('./lint-output.json');\n" \
"\n" \
"test('it should produce the expected lint ouput', () => {\n" \
" expect(actualLintOutput).toMatchSnapshot();\n" \
"});\n" > test.js

# Run the integration test
CMD [ "./test.sh" ]