Skip to content

add missing imports source action and tests #2744

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { internalHelpers } from 'svelte2tsx';
import ts from 'typescript';
import {
CancellationToken,
Expand All @@ -24,6 +25,7 @@ import {
} from '../../../lib/documents';
import { LSConfigManager } from '../../../ls-config';
import {
createGetCanonicalFileName,
flatten,
getIndent,
isNotNullOrUndefined,
Expand All @@ -37,13 +39,15 @@ import {
import { CodeActionsProvider } from '../../interfaces';
import { DocumentSnapshot, SvelteDocumentSnapshot } from '../DocumentSnapshot';
import { LSAndTSDocResolver } from '../LSAndTSDocResolver';
import { LanguageServiceContainer } from '../service';
import {
changeSvelteComponentName,
convertRange,
isInScript,
toGeneratedSvelteComponentName
} from '../utils';
import { CompletionsProviderImpl } from './CompletionProvider';
import { DiagnosticCode } from './DiagnosticsProvider';
import {
findClosestContainingNode,
FormatCodeBasis,
Expand All @@ -53,15 +57,12 @@ import {
isTextSpanInGeneratedCode,
SnapshotMap
} from './utils';
import { DiagnosticCode } from './DiagnosticsProvider';
import { createGetCanonicalFileName } from '../../../utils';
import { LanguageServiceContainer } from '../service';
import { internalHelpers } from 'svelte2tsx';

/**
* TODO change this to protocol constant if it's part of the protocol
*/
export const SORT_IMPORT_CODE_ACTION_KIND = 'source.sortImports';
export const ADD_MISSING_IMPORTS_CODE_ACTION_KIND = 'source.addMissingImports';

interface RefactorArgs {
type: 'refactor';
Expand Down Expand Up @@ -121,6 +122,10 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
);
}

if (context.only?.[0] === ADD_MISSING_IMPORTS_CODE_ACTION_KIND) {
return await this.addMissingImports(document, cancellationToken);
}

// for source action command (all source.xxx)
// vscode would show different source code action kinds to choose from
if (context.only?.[0] === CodeActionKind.Source) {
Expand All @@ -130,7 +135,8 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
document,
cancellationToken,
/**skipDestructiveCodeActions */ true
))
)),
...(await this.addMissingImports(document, cancellationToken))
];
}

Expand Down Expand Up @@ -1553,4 +1559,48 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
private async getLSAndTSDoc(document: Document) {
return this.lsAndTsDocResolver.getLSAndTSDoc(document);
}

private async addMissingImports(
document: Document,
cancellationToken?: CancellationToken
): Promise<CodeAction[]> {
// Re-introduce LS/TSDoc resolution and diagnostic check
const { lang, tsDoc } = await this.getLSAndTSDoc(document);
if (cancellationToken?.isCancellationRequested) {
return [];
}

// Check if there are any relevant "cannot find name" diagnostics
const diagnostics = lang.getSemanticDiagnostics(tsDoc.filePath);
const hasMissingImports = diagnostics.some(
(diag) =>
(diag.code === DiagnosticCode.CANNOT_FIND_NAME ||
diag.code === DiagnosticCode.CANNOT_FIND_NAME_X_DID_YOU_MEAN_Y) &&
// Ensure the diagnostic is not in generated code
!isTextSpanInGeneratedCode(tsDoc.getFullText(), {
start: diag.start ?? 0,
length: diag.length ?? 0
})
);

// Only return the action if there are potential imports to add
if (!hasMissingImports) {
return [];
}

// If imports might be needed, create the deferred action
const codeAction = CodeAction.create(
FIX_IMPORT_FIX_DESCRIPTION,
ADD_MISSING_IMPORTS_CODE_ACTION_KIND
);

const data: QuickFixAllResolveInfo = {
uri: document.uri,
fixName: FIX_IMPORT_FIX_NAME,
fixId: FIX_IMPORT_FIX_ID
};
codeAction.data = data;

return [codeAction];
}
}
6 changes: 5 additions & 1 deletion packages/language-server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ import { debounceThrottle, isNotNullOrUndefined, normalizeUri, urlToPath } from
import { FallbackWatcher } from './lib/FallbackWatcher';
import { configLoader } from './lib/documents/configLoader';
import { setIsTrusted } from './importPackage';
import { SORT_IMPORT_CODE_ACTION_KIND } from './plugins/typescript/features/CodeActionsProvider';
import {
SORT_IMPORT_CODE_ACTION_KIND,
ADD_MISSING_IMPORTS_CODE_ACTION_KIND
} from './plugins/typescript/features/CodeActionsProvider';
import { createLanguageServices } from './plugins/css/service';
import { FileSystemProvider } from './plugins/css/FileSystemProvider';

Expand Down Expand Up @@ -270,6 +273,7 @@ export function startServer(options?: LSOptions) {
CodeActionKind.QuickFix,
CodeActionKind.SourceOrganizeImports,
SORT_IMPORT_CODE_ACTION_KIND,
ADD_MISSING_IMPORTS_CODE_ACTION_KIND,
...(clientSupportApplyEditCommand ? [CodeActionKind.Refactor] : [])
].filter(
clientSupportedCodeActionKinds &&
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import * as assert from 'assert';
import * as path from 'path';
import { VERSION } from 'svelte/compiler';
import { internalHelpers } from 'svelte2tsx';
import ts from 'typescript';
import {
CancellationTokenSource,
Expand All @@ -12,17 +14,16 @@ import {
import { Document, DocumentManager } from '../../../../src/lib/documents';
import { LSConfigManager } from '../../../../src/ls-config';
import {
ADD_MISSING_IMPORTS_CODE_ACTION_KIND,
CodeActionsProviderImpl,
SORT_IMPORT_CODE_ACTION_KIND
} from '../../../../src/plugins/typescript/features/CodeActionsProvider';
import { CompletionsProviderImpl } from '../../../../src/plugins/typescript/features/CompletionProvider';
import { DiagnosticCode } from '../../../../src/plugins/typescript/features/DiagnosticsProvider';
import { LSAndTSDocResolver } from '../../../../src/plugins/typescript/LSAndTSDocResolver';
import { __resetCache } from '../../../../src/plugins/typescript/service';
import { pathToUrl } from '../../../../src/utils';
import { recursiveServiceWarmup } from '../test-utils';
import { DiagnosticCode } from '../../../../src/plugins/typescript/features/DiagnosticsProvider';
import { VERSION } from 'svelte/compiler';
import { internalHelpers } from 'svelte2tsx';

const testDir = path.join(__dirname, '..');
const indent = ' '.repeat(4);
Expand Down Expand Up @@ -2229,4 +2230,84 @@ describe('CodeActionsProvider', function () {
after(() => {
__resetCache();
});

it('provides source action for adding all missing imports', async () => {
const { provider, document } = setup('codeaction-custom-fix-all-component5.svelte');

const range = Range.create(Position.create(4, 1), Position.create(4, 15));

// Request the specific source action
const codeActions = await provider.getCodeActions(document, range, {
diagnostics: [], // Diagnostics might not be needed here if we only want the source action by kind
only: [ADD_MISSING_IMPORTS_CODE_ACTION_KIND]
});

assert.ok(codeActions.length > 0, 'No code actions found');

// Find the action by its kind
const addImportsAction = codeActions.find((action) => action.data);

// Ensure the action was found and has data (as it's now deferred)
assert.ok(addImportsAction, 'Add missing imports action should be found');
assert.ok(
addImportsAction.data,
'Add missing imports action should have data for resolution'
);

// Resolve the action to get the edits
const resolvedAction = await provider.resolveCodeAction(document, addImportsAction);

// Assert the edits on the resolved action
assert.ok(resolvedAction.edit, 'Resolved action should have an edit');
(<TextDocumentEdit>resolvedAction.edit?.documentChanges?.[0])?.edits.forEach(
(edit) => (edit.newText = harmonizeNewLines(edit.newText))
);

assert.deepStrictEqual(resolvedAction.edit, {
documentChanges: [
{
edits: [
{
newText:
`\n${indent}import FixAllImported from \"./importing/FixAllImported.svelte\";\n` +
`${indent}import FixAllImported2 from \"./importing/FixAllImported2.svelte\";\n`,
range: {
start: {
character: 18,
line: 0
},
end: {
character: 18,
line: 0
}
}
}
],
textDocument: {
uri: getUri('codeaction-custom-fix-all-component5.svelte'),
version: null
}
}
]
});

// Optional: Verify the kind and title remain correct on the resolved action
assert.strictEqual(resolvedAction.kind, ADD_MISSING_IMPORTS_CODE_ACTION_KIND);
assert.strictEqual(resolvedAction.title, 'Add all missing imports');
});

it('provides source action for adding all missing imports only when imports are missing', async () => {
const { provider, document } = setup('codeaction-custom-fix-all-component6.svelte');

const codeActions = await provider.getCodeActions(
document,
Range.create(Position.create(1, 4), Position.create(1, 5)),
{
diagnostics: [], // No diagnostics = no missing imports
only: [ADD_MISSING_IMPORTS_CODE_ACTION_KIND]
}
);

assert.deepStrictEqual(codeActions, []);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script lang="ts">

</script>

<FixAllImported />
<FixAllImported2 />

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<script lang="ts">
import FixAllImported from './importing/FixAllImported.svelte';
import FixAllImported2 from './importing/FixAllImported2.svelte';


</script>

<FixAllImported />
<FixAllImported2 />