Skip to content

Fix svelte plugin diagnostic mapping for context="module" #587

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions packages/language-server/src/plugins/svelte/SvelteDocument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,11 +175,16 @@ export class SvelteFragmentMapper {
}

static async createScript(originalDoc: Document, transpiled: string, processed: Processed[]) {
const scriptInfo = originalDoc.scriptInfo || originalDoc.moduleScriptInfo;
const maybeScriptTag = extractScriptTags(transpiled);
const maybeScriptTagInfo =
maybeScriptTag && (maybeScriptTag.script || maybeScriptTag.moduleScript);

return SvelteFragmentMapper.create(
originalDoc,
transpiled,
originalDoc.scriptInfo,
extractScriptTags(transpiled)?.script || null,
scriptInfo,
maybeScriptTagInfo || null,
processed,
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,10 @@ async function createParserErrorDiagnostic(error: any, document: Document) {

if (diagnostic.message.includes('expected')) {
const isInStyle = isInTag(diagnostic.range.start, document.styleInfo);
const isInScript = isInTag(diagnostic.range.start, document.scriptInfo);
const isInScript = isInTag(
diagnostic.range.start,
document.scriptInfo || document.moduleScriptInfo,
);

if (isInStyle || isInScript) {
diagnostic.message +=
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ function preprocessSvelteFile(document: Document, options: SvelteSnapshotOptions
if (tsxMap) {
tsxMap.sources = [document.uri];

const tsCheck = getTsCheckComment(document.scriptInfo?.content);
const scriptInfo = document.scriptInfo || document.moduleScriptInfo;
const tsCheck = getTsCheckComment(scriptInfo?.content);
if (tsCheck) {
text = tsCheck + text;
nrPrependedLines = 1;
Expand All @@ -172,7 +173,8 @@ function preprocessSvelteFile(document: Document, options: SvelteSnapshotOptions
};

// fall back to extracted script, if any
text = document.scriptInfo ? document.scriptInfo.content : '';
const scriptInfo = document.scriptInfo || document.moduleScriptInfo;
text = scriptInfo ? scriptInfo.content : '';
}

return {
Expand Down Expand Up @@ -330,7 +332,7 @@ export class SvelteSnapshotFragment implements SnapshotFragment {
) {}

get scriptInfo() {
return this.parent.scriptInfo;
return this.parent.scriptInfo || this.parent.moduleScriptInfo;
}

getOriginalPosition(pos: Position): Position {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
}

private async organizeImports(document: Document): Promise<CodeAction[]> {
if (!document.scriptInfo) {
if (!document.scriptInfo && !document.moduleScriptInfo) {
return [];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export interface CompletionEntryWithIdentifer extends ts.CompletionEntry, TextDo
type validTriggerCharacter = '.' | '"' | "'" | '`' | '/' | '@' | '<' | '#';

export class CompletionsProviderImpl implements CompletionsProvider<CompletionEntryWithIdentifer> {
constructor(private readonly lsAndTsDocResolver: LSAndTSDocResolver) { }
constructor(private readonly lsAndTsDocResolver: LSAndTSDocResolver) {}

/**
* The language service throws an error if the character is not a valid trigger character.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import * as assert from 'assert';
import * as path from 'path';
import * as fs from 'fs';
import { Diagnostic, DiagnosticSeverity, Position } from 'vscode-languageserver';
import { Document } from '../../../../src/lib/documents';
import { getDiagnostics } from '../../../../src/plugins/svelte/features/getDiagnostics';
Expand All @@ -7,7 +9,9 @@ import {
TranspileErrorSource,
} from '../../../../src/plugins/svelte/SvelteDocument';
import { SvelteConfig } from '../../../../src/lib/documents/configLoader';
import { CompilerWarningsSettings } from '../../../../src/ls-config';
import { CompilerWarningsSettings, LSConfigManager } from '../../../../src/ls-config';
import { pathToUrl } from '../../../../src/utils';
import { SveltePlugin } from '../../../../src/plugins';

describe('SveltePlugin#getDiagnostics', () => {
async function expectDiagnosticsFor({
Expand All @@ -31,6 +35,15 @@ describe('SveltePlugin#getDiagnostics', () => {
};
}

function setupFromFile(filename: string) {
const testDir = path.join(__dirname, '..');
const filePath = path.join(testDir, 'testfiles', filename);
const document = new Document(pathToUrl(filePath), fs.readFileSync(filePath, 'utf-8'));
const pluginManager = new LSConfigManager();
const plugin = new SveltePlugin(pluginManager);
return { plugin, document };
}

it('expect svelte.config.js error', async () => {
(
await expectDiagnosticsFor({
Expand Down Expand Up @@ -369,4 +382,36 @@ describe('SveltePlugin#getDiagnostics', () => {
},
]);
});

it('should correctly determine diagnostic position', async () => {
const { plugin, document } = setupFromFile('diagnostics.svelte');
const diagnostics = await plugin.getDiagnostics(document);

assert.deepStrictEqual(diagnostics, [
{
range: { start: { line: 1, character: 15 }, end: { line: 1, character: 27 } },
message:
"Component has unused export property 'name'. If it is for external reference only, please consider using `export const name`",
severity: 2,
source: 'svelte',
code: 'unused-export-let',
},
]);
});

it('should correctly determine diagnostic position for context="module"', async () => {
const { plugin, document } = setupFromFile('diagnostics-module.svelte');
const diagnostics = await plugin.getDiagnostics(document);

assert.deepStrictEqual(diagnostics, [
{
range: { start: { line: 1, character: 15 }, end: { line: 1, character: 27 } },
message:
"Component has unused export property 'name'. If it is for external reference only, please consider using `export const name`",
severity: 2,
source: 'svelte',
code: 'unused-export-let',
},
]);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<script context="module" lang="typescript">
export let name: string;
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<script lang="typescript">
export let name: string;
</script>