From 2d17c872b4976137252fbda9fe3e7d82cf17e560 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 18 Feb 2021 16:54:19 +0100 Subject: [PATCH 1/5] Silence some wrong unsafe-member-access errors Fixes #87 .. but not all possible cases, see README. --- README.md | 18 ++++ src/preprocess.js | 47 ++++++---- .../.eslintrc.js | 17 ++++ .../Input.svelte | 40 +++++++++ .../expected.json | 90 +++++++++++++++++++ .../external-file.ts | 2 + .../tsconfig.json | 3 + test/samples/typescript/expected.json | 16 ++-- 8 files changed, 206 insertions(+), 27 deletions(-) create mode 100644 test/samples/typescript-unsafe-member-access/.eslintrc.js create mode 100644 test/samples/typescript-unsafe-member-access/Input.svelte create mode 100644 test/samples/typescript-unsafe-member-access/expected.json create mode 100644 test/samples/typescript-unsafe-member-access/external-file.ts create mode 100644 test/samples/typescript-unsafe-member-access/tsconfig.json diff --git a/README.md b/README.md index ae77479..0ddd962 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,24 @@ module.exports = { }; ``` +Note that there are some limitations to these type-aware rules currently. Specifically, checks for `no-unsafe-member-access` / `no-unsafe-call` will report false positives for reactive assignments and store subscriptions: + +```svelte + +``` + ## Interactions with other plugins Care needs to be taken when using this plugin alongside others. Take a look at [this list of things you need to watch out for](OTHER_PLUGINS.md). diff --git a/src/preprocess.js b/src/preprocess.js index dbb2114..1fb19c4 100644 --- a/src/preprocess.js +++ b/src/preprocess.js @@ -111,7 +111,12 @@ export const preprocess = text => { const block = new_block(); state.blocks.set(with_file_ending('instance'), block); - block.transformed_code = vars.filter(v => v.injected || v.module).map(v => `let ${v.name};`).join(''); + if (ast.module && processor_options.typescript) { + block.transformed_code = vars.filter(v => v.injected).map(v => `let ${v.name};`).join(''); + block.transformed_code += text.slice(ast.module.content.start, ast.module.content.end); + } else { + block.transformed_code = vars.filter(v => v.injected || v.module).map(v => `let ${v.name};`).join(''); + } get_translation(text, block, ast.instance.content); @@ -123,7 +128,19 @@ export const preprocess = text => { const block = new_block(); state.blocks.set(with_file_ending('template'), block); - block.transformed_code = vars.map(v => `let ${v.name};`).join(''); + if (processor_options.typescript) { + block.transformed_code = ''; + if (ast.module) { + block.transformed_code += text.slice(ast.module.content.start, ast.module.content.end); + } + if (ast.instance) { + block.transformed_code += '\n'; + block.transformed_code += vars.filter(v => v.injected).map(v => `let ${v.name};`).join(''); + block.transformed_code += text.slice(ast.instance.content.start, ast.instance.content.end); + } + } else { + block.transformed_code = vars.map(v => `let ${v.name};`).join(''); + } const nodes_with_contextual_scope = new WeakSet(); let in_quoted_attribute = false; @@ -209,14 +226,10 @@ const ts_import_transformer = (context) => { // 5. parse the source to get the AST // 6. return AST of step 5, warnings and vars of step 2 function compile_code(text, compiler, processor_options) { - let ast; - let warnings; - let vars; - const ts = processor_options.typescript; - let mapper; - let ts_result; - if (ts) { + if (!ts) { + return compiler.compile(text, { generate: false, ...processor_options.compiler_options }); + } else { const diffs = []; let accumulated_diff = 0; const transpiled = text.replace(/([^]*?)<\/script>/gi, (match, attributes = '', content) => { @@ -245,7 +258,9 @@ function compile_code(text, compiler, processor_options) { }); return `${output.outputText}`; }); - mapper = new DocumentMapper(text, transpiled, diffs); + const mapper = new DocumentMapper(text, transpiled, diffs); + + let ts_result; try { ts_result = compiler.compile(transpiled, { generate: false, ...processor_options.compiler_options }); } catch (err) { @@ -263,16 +278,10 @@ function compile_code(text, compiler, processor_options) { .replace(/[^\n][^\n][^\n][^\n]\n/g, '/**/\n') }`; }); - } - - if (!ts) { - ({ ast, warnings, vars } = compiler.compile(text, { generate: false, ...processor_options.compiler_options })); - } else { // if we do a full recompile Svelte can fail due to the blank script tag not declaring anything // so instead we just parse for the AST (which is likely faster, anyways) - ast = compiler.parse(text, { ...processor_options.compiler_options }); - ({ warnings, vars } = ts_result); + const ast = compiler.parse(text, { ...processor_options.compiler_options }); + const{ warnings, vars } = ts_result; + return { ast, warnings, vars, mapper }; } - - return { ast, warnings, vars, mapper }; } diff --git a/test/samples/typescript-unsafe-member-access/.eslintrc.js b/test/samples/typescript-unsafe-member-access/.eslintrc.js new file mode 100644 index 0000000..ce678f9 --- /dev/null +++ b/test/samples/typescript-unsafe-member-access/.eslintrc.js @@ -0,0 +1,17 @@ +module.exports = { + parser: "@typescript-eslint/parser", + plugins: [ + "@typescript-eslint", + ], + parserOptions: { + project: ["./tsconfig.json"], + tsconfigRootDir: __dirname, + extraFileExtensions: [".svelte"], + }, + settings: { + "svelte3/typescript": require("typescript"), + }, + rules: { + "@typescript-eslint/no-unsafe-member-access": "error", + }, +}; diff --git a/test/samples/typescript-unsafe-member-access/Input.svelte b/test/samples/typescript-unsafe-member-access/Input.svelte new file mode 100644 index 0000000..b1d4411 --- /dev/null +++ b/test/samples/typescript-unsafe-member-access/Input.svelte @@ -0,0 +1,40 @@ + + + + +{context_safe.length} +{context_unsafe.length} +{external_safe.length} +{external_unsafe.length} +{instance_safe.length} +{instance_unsafe.length} + +{reactive_unsafe.length} + +{$writable_unsafe.length} diff --git a/test/samples/typescript-unsafe-member-access/expected.json b/test/samples/typescript-unsafe-member-access/expected.json new file mode 100644 index 0000000..796f50c --- /dev/null +++ b/test/samples/typescript-unsafe-member-access/expected.json @@ -0,0 +1,90 @@ +[ + { + "ruleId": "@typescript-eslint/no-unsafe-member-access", + "severity": 2, + "line": 6, + "column": 17, + "endLine": 6, + "endColumn": 38 + }, + { + "ruleId": "@typescript-eslint/no-unsafe-member-access", + "severity": 2, + "line": 20, + "column": 14, + "endLine": 20, + "endColumn": 35 + }, + { + "ruleId": "@typescript-eslint/no-unsafe-member-access", + "severity": 2, + "line": 22, + "column": 14, + "endLine": 22, + "endColumn": 36 + }, + { + "ruleId": "@typescript-eslint/no-unsafe-member-access", + "severity": 2, + "line": 24, + "column": 17, + "endLine": 24, + "endColumn": 39 + }, + { + "ruleId": "@typescript-eslint/no-unsafe-member-access", + "severity": 2, + "line": 26, + "column": 17, + "endLine": 26, + "endColumn": 39 + }, + { + "ruleId": "@typescript-eslint/no-unsafe-member-access", + "severity": 2, + "line": 28, + "column": 17, + "endLine": 28, + "endColumn": 40 + }, + { + "ruleId": "@typescript-eslint/no-unsafe-member-access", + "severity": 2, + "line": 32, + "column": 2, + "endLine": 32, + "endColumn": 23 + }, + { + "ruleId": "@typescript-eslint/no-unsafe-member-access", + "severity": 2, + "line": 34, + "column": 2, + "endLine": 34, + "endColumn": 24 + }, + { + "ruleId": "@typescript-eslint/no-unsafe-member-access", + "severity": 2, + "line": 36, + "column": 2, + "endLine": 36, + "endColumn": 24 + }, + { + "ruleId": "@typescript-eslint/no-unsafe-member-access", + "severity": 2, + "line": 38, + "column": 2, + "endLine": 38, + "endColumn": 24 + }, + { + "ruleId": "@typescript-eslint/no-unsafe-member-access", + "severity": 2, + "line": 40, + "column": 2, + "endLine": 40, + "endColumn": 25 + } +] \ No newline at end of file diff --git a/test/samples/typescript-unsafe-member-access/external-file.ts b/test/samples/typescript-unsafe-member-access/external-file.ts new file mode 100644 index 0000000..16ac2a7 --- /dev/null +++ b/test/samples/typescript-unsafe-member-access/external-file.ts @@ -0,0 +1,2 @@ +export const external_safe = ['hi']; +export const external_unsafe: any = null; diff --git a/test/samples/typescript-unsafe-member-access/tsconfig.json b/test/samples/typescript-unsafe-member-access/tsconfig.json new file mode 100644 index 0000000..9aadfdc --- /dev/null +++ b/test/samples/typescript-unsafe-member-access/tsconfig.json @@ -0,0 +1,3 @@ +{ + "include": ["**/*"] +} diff --git a/test/samples/typescript/expected.json b/test/samples/typescript/expected.json index 8c05a26..e598e26 100644 --- a/test/samples/typescript/expected.json +++ b/test/samples/typescript/expected.json @@ -35,8 +35,8 @@ "messageId": "suggestUnknown", "fix": { "range": [ - 29, - 32 + 58, + 61 ], "text": "unknown" }, @@ -46,8 +46,8 @@ "messageId": "suggestNever", "fix": { "range": [ - 29, - 32 + 58, + 61 ], "text": "never" }, @@ -82,8 +82,8 @@ "messageId": "suggestUnknown", "fix": { "range": [ - 50, - 53 + 79, + 82 ], "text": "unknown" }, @@ -93,8 +93,8 @@ "messageId": "suggestNever", "fix": { "range": [ - 50, - 53 + 79, + 82 ], "text": "never" }, From 0edbcbca9f9473351bf8b295c2f9e46bb1e22acf Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 18 Feb 2021 16:58:02 +0100 Subject: [PATCH 2/5] tabs --- .../Input.svelte | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/test/samples/typescript-unsafe-member-access/Input.svelte b/test/samples/typescript-unsafe-member-access/Input.svelte index b1d4411..25edda1 100644 --- a/test/samples/typescript-unsafe-member-access/Input.svelte +++ b/test/samples/typescript-unsafe-member-access/Input.svelte @@ -2,30 +2,30 @@ const context_safe = []; const context_unsafe: any = null; - console.log(context_safe.length); - console.log(context_unsafe.length); + console.log(context_safe.length); + console.log(context_unsafe.length); {context_safe.length} From 1a916e28f7d2b18aeb095ff11fad66a88d1cc2b3 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 18 Feb 2021 17:11:36 +0100 Subject: [PATCH 3/5] adjust columns --- .../expected.json | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/samples/typescript-unsafe-member-access/expected.json b/test/samples/typescript-unsafe-member-access/expected.json index 796f50c..413f59f 100644 --- a/test/samples/typescript-unsafe-member-access/expected.json +++ b/test/samples/typescript-unsafe-member-access/expected.json @@ -3,9 +3,9 @@ "ruleId": "@typescript-eslint/no-unsafe-member-access", "severity": 2, "line": 6, - "column": 17, + "column": 14, "endLine": 6, - "endColumn": 38 + "endColumn": 35 }, { "ruleId": "@typescript-eslint/no-unsafe-member-access", @@ -27,25 +27,25 @@ "ruleId": "@typescript-eslint/no-unsafe-member-access", "severity": 2, "line": 24, - "column": 17, + "column": 14, "endLine": 24, - "endColumn": 39 + "endColumn": 36 }, { "ruleId": "@typescript-eslint/no-unsafe-member-access", "severity": 2, "line": 26, - "column": 17, + "column": 14, "endLine": 26, - "endColumn": 39 + "endColumn": 36 }, { "ruleId": "@typescript-eslint/no-unsafe-member-access", "severity": 2, "line": 28, - "column": 17, + "column": 14, "endLine": 28, - "endColumn": 40 + "endColumn": 37 }, { "ruleId": "@typescript-eslint/no-unsafe-member-access", From 308efbb95c2d4ded3382f1c65f37e7b84f3588d4 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Fri, 19 Feb 2021 14:28:55 +0100 Subject: [PATCH 4/5] make limitation notes more general --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 0ddd962..c1049fe 100644 --- a/README.md +++ b/README.md @@ -102,7 +102,7 @@ module.exports = { }; ``` -Note that there are some limitations to these type-aware rules currently. Specifically, checks for `no-unsafe-member-access` / `no-unsafe-call` will report false positives for reactive assignments and store subscriptions: +Note that there are some limitations to these type-aware rules currently. Specifically, checks in the context of reactive assignments and store subscriptions will report false positives or false negatives, depending on the rule. For reactive assignments, you can work around this by explicitely typing the reactive assignment. An example with the `no-unsafe-member-access` rule: ```svelte