Skip to content

Fixing namespace import debug failure #59004

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.

Sign up for GitHub

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

Closed
wants to merge 17 commits into from
Closed

Fixing namespace import debug failure #59004

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
e31bb29
Fixes namespace debug failure but test 2 fails
navya9singh Jun 20, 2024
c4d86b8
fixing test 2
navya9singh Jun 24, 2024
1347b9f
fixing formatting
navya9singh Jun 24, 2024
45af3f0
fixing baselines
navya9singh Jun 24, 2024
9e4a30a
Merge branch 'main' of https://github.com/microsoft/TypeScript into e…
navya9singh Jun 24, 2024
e35b0f7
Adding more fixes and tests
navya9singh Jun 27, 2024
c0aaae4
Removing previous changes
navya9singh Jun 27, 2024
dc452e8
deleting old baselines
navya9singh Jun 27, 2024
d47ff25
default import fixes
navya9singh Jul 11, 2024
ba4c3dd
fixing baselines
navya9singh Jul 11, 2024
fb453ee
changing baselines
navya9singh Jul 11, 2024
ba364e8
Merge branch 'main' of https://github.com/microsoft/TypeScript into e…
navya9singh Jul 11, 2024
b21b4a1
fixing tests and baselines
navya9singh Jul 15, 2024
b23e6d8
removing baseline for deleted test
navya9singh Jul 15, 2024
cb06e8f
fixes
navya9singh Jul 18, 2024
2a432e4
addressing pr comments
navya9singh Jul 20, 2024
e3deb17
addAsTypeOnly changes
navya9singh Jul 22, 2024
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
53 changes: 36 additions & 17 deletions src/services/codefixes/importFixes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,31 +274,50 @@ function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, prog
}

function addImportFromExportedSymbol(exportedSymbol: Symbol, isValidTypeOnlyUseSite?: boolean, referenceImport?: ImportOrRequireAliasDeclaration) {
const moduleSymbol = Debug.checkDefined(exportedSymbol.parent);
const symbolName = getNameForExportedSymbol(exportedSymbol, getEmitScriptTarget(compilerOptions));
const checker = program.getTypeChecker();
const symbol = checker.getMergedSymbol(skipAlias(exportedSymbol, checker));
const exportInfo = getAllExportInfoForSymbol(sourceFile, symbol, symbolName, moduleSymbol, /*preferCapitalized*/ false, program, host, preferences, cancellationToken);
const useRequire = shouldUseRequire(sourceFile, program);
if (referenceImport && isNamespaceImport(referenceImport) && exportedSymbol.valueDeclaration) {
const namespacePrefix = getNamespaceLikeImportText(referenceImport.parent.parent);
const moduleSpecifier = moduleSpecifiers.getLocalModuleSpecifierBetweenFileNames(
sourceFile,
exportedSymbol.valueDeclaration.getSourceFile().fileName,
compilerOptions,
createModuleSpecifierResolutionHost(program, host),
);
if (namespacePrefix) {
const info: FixInfo = {
fix: { kind: ImportFixKind.AddNew, importKind: ImportKind.Namespace, addAsTypeOnly: isTypeOnlyImportDeclaration(referenceImport) ? AddAsTypeOnly.Required : AddAsTypeOnly.NotAllowed, useRequire, moduleSpecifierKind: undefined, moduleSpecifier },
symbolName: namespacePrefix,
errorIdentifierText: undefined,
};
addImport(info);
}
}
else {
const moduleSymbol = Debug.checkDefined(exportedSymbol.parent);
const symbolName = getNameForExportedSymbol(exportedSymbol, getEmitScriptTarget(compilerOptions));
const symbol = checker.getMergedSymbol(skipAlias(exportedSymbol, checker));
const exportInfo = getAllExportInfoForSymbol(sourceFile, symbol, symbolName, moduleSymbol, /*preferCapitalized*/ false, program, host, preferences, cancellationToken);
if (!exportInfo) {
// If no exportInfo is found, this means export could not be resolved when we have filtered for autoImportFileExcludePatterns,
// so we should not generate an import.
Debug.assert(preferences.autoImportFileExcludePatterns?.length);
return;
}
const useRequire = shouldUseRequire(sourceFile, program);
let fix = getImportFixForSymbol(sourceFile, exportInfo, program, /*position*/ undefined, !!isValidTypeOnlyUseSite, useRequire, host, preferences);
if (fix) {
const localName = tryCast(referenceImport?.name, isIdentifier)?.text ?? symbolName;
if (
referenceImport
&& isTypeOnlyImportDeclaration(referenceImport)
&& (fix.kind === ImportFixKind.AddNew || fix.kind === ImportFixKind.AddToExisting)
&& fix.addAsTypeOnly === AddAsTypeOnly.Allowed
) {
// Copy the type-only status from the reference import
fix = { ...fix, addAsTypeOnly: AddAsTypeOnly.Required };
let fix = getImportFixForSymbol(sourceFile, exportInfo, program, /*position*/ undefined, !!isValidTypeOnlyUseSite, useRequire, host, preferences);
if (fix) {
const localName = tryCast(referenceImport?.name, isIdentifier)?.text ?? symbolName;
if (
referenceImport
&& isTypeOnlyImportDeclaration(referenceImport)
&& (fix.kind === ImportFixKind.AddNew || fix.kind === ImportFixKind.AddToExisting)
&& fix.addAsTypeOnly === AddAsTypeOnly.Allowed
) {
// Copy the type-only status from the reference import
fix = { ...fix, addAsTypeOnly: AddAsTypeOnly.Required };
}
addImport({ fix, symbolName: localName ?? symbolName, errorIdentifierText: undefined });
}
addImport({ fix, symbolName: localName ?? symbolName, errorIdentifierText: undefined });
}
}

Expand Down
14 changes: 11 additions & 3 deletions src/services/refactors/moveToFile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1131,14 +1131,22 @@ export function getExistingLocals(sourceFile: SourceFile, statements: readonly S
const declaration = importFromModuleSpecifier(moduleSpecifier);
if (
isImportDeclaration(declaration) && declaration.importClause &&
declaration.importClause.namedBindings && isNamedImports(declaration.importClause.namedBindings)
declaration.importClause.namedBindings
) {
for (const e of declaration.importClause.namedBindings.elements) {
const symbol = checker.getSymbolAtLocation(e.propertyName || e.name);
if (isNamespaceImport(declaration.importClause.namedBindings)) {
const symbol = checker.getSymbolAtLocation(declaration.importClause.namedBindings.name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there’s no need for checker.getSymbolAtLocation anywhere in this function. Declarations always have a .symbol; you can just do declaration.importClause.namedBindings.symbol here, and .symbol on each ImportSpecifier below. Even the existing variable declaration / binding pattern stuff further down can lose the getSymbolAtLocation.

if (symbol) {
existingLocals.add(skipAlias(symbol, checker));
}
}
else if (isNamedImports(declaration.importClause.namedBindings)) {
for (const e of declaration.importClause.namedBindings.elements) {
const symbol = checker.getSymbolAtLocation(e.propertyName || e.name);
if (symbol) {
existingLocals.add(skipAlias(symbol, checker));
}
}
}
}
if (isVariableDeclarationInitializedToRequire(declaration.parent) && isObjectBindingPattern(declaration.parent.name)) {
for (const e of declaration.parent.name.elements) {
Expand Down
Loading
Loading