Fixed bug in completion provider that resulted in an attempt to token…

…ize and parse a binary (native) library file. This led to a crash in some cases. This addresses microsoft/pylance-release#5090.
microsoft · Nov 7, 2023 · e519173 · e519173
1 parent f8746ed
commit e519173
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 15 deletions.
diff --git a/packages/pyright-internal/src/analyzer/importResolver.ts b/packages/pyright-internal/src/analyzer/importResolver.ts
@@ -103,8 +103,8 @@ interface SupportedVersionRange {
 }
 
 const supportedNativeLibExtensions = ['.pyd', '.so', '.dylib'];
-export const supportedSourceFileExtensions = ['.py', '.pyi'];
-export const supportedFileExtensions = [...supportedSourceFileExtensions, ...supportedNativeLibExtensions];
+const supportedSourceFileExtensions = ['.py', '.pyi'];
+const supportedFileExtensions = [...supportedSourceFileExtensions, ...supportedNativeLibExtensions];
 
 // Should we allow partial resolution for third-party packages? Some use tricks
 // to populate their package namespaces, so we might be able to partially resolve
@@ -139,6 +139,16 @@ export class ImportResolver {
         return this.serviceProvider.tryGet(ServiceKeys.partialStubs);
     }
 
+    static isSupportedImportSourceFile(path: string) {
+        const fileExtension = getFileExtension(path).toLowerCase();
+        return supportedSourceFileExtensions.some((ext) => fileExtension === ext);
+    }
+
+    static isSupportedImportFile(path: string) {
+        const fileExtension = getFileExtension(path).toLowerCase();
+        return supportedFileExtensions.some((ext) => fileExtension === ext);
+    }
+
     invalidateCache() {
         this._cachedImportResults = new Map<string | undefined, CachedImportResults>();
         this._cachedModuleNameResults = new Map<string, Map<string, ModuleImportInfo>>();
@@ -2396,10 +2406,9 @@ export class ImportResolver {
         entries.files.forEach((file) => {
             // Strip multi-dot extensions to handle file names like "foo.cpython-32m.so". We want
             // to detect the ".so" but strip off the entire ".cpython-32m.so" extension.
-            const fileExtension = getFileExtension(file, /* multiDotExtension */ false).toLowerCase();
             const fileWithoutExtension = stripFileExtension(file, /* multiDotExtension */ true);
 
-            if (supportedFileExtensions.some((ext) => ext === fileExtension)) {
+            if (ImportResolver.isSupportedImportFile(file)) {
                 if (fileWithoutExtension === '__init__') {
                     return;
                 }

diff --git a/packages/pyright-internal/src/analyzer/service.ts b/packages/pyright-internal/src/analyzer/service.ts
@@ -30,7 +30,6 @@ import {
     containsPath,
     forEachAncestorDirectory,
     getDirectoryPath,
-    getFileExtension,
     getFileName,
     getFileSpec,
     getFileSystemEntries,
@@ -57,12 +56,7 @@ import {
     BackgroundAnalysisProgramFactory,
     InvalidatedReason,
 } from './backgroundAnalysisProgram';
-import {
-    ImportResolver,
-    ImportResolverFactory,
-    createImportedModuleDescriptor,
-    supportedSourceFileExtensions,
-} from './importResolver';
+import { ImportResolver, ImportResolverFactory, createImportedModuleDescriptor } from './importResolver';
 import { MaxAnalysisTime, Program } from './program';
 import { findPythonSearchPaths } from './pythonPathUtils';
 import { IPythonMode } from './sourceFile';
@@ -1093,8 +1087,7 @@ export class AnalyzerService {
 
                 // Add the implicit import paths.
                 importResult.filteredImplicitImports.forEach((implicitImport) => {
-                    const fileExtension = getFileExtension(implicitImport.path).toLowerCase();
-                    if (supportedSourceFileExtensions.some((ext) => fileExtension === ext)) {
+                    if (ImportResolver.isSupportedImportSourceFile(implicitImport.path)) {
                         filesToImport.push(implicitImport.path);
                     }
                 });

diff --git a/packages/pyright-internal/src/languageService/completionProvider.ts b/packages/pyright-internal/src/languageService/completionProvider.ts
@@ -30,7 +30,7 @@ import {
 } from '../analyzer/declaration';
 import { isDefinedInFile } from '../analyzer/declarationUtils';
 import { convertDocStringToMarkdown, convertDocStringToPlainText } from '../analyzer/docStringConversion';
-import { ImportedModuleDescriptor } from '../analyzer/importResolver';
+import { ImportedModuleDescriptor, ImportResolver } from '../analyzer/importResolver';
 import { ImportResult } from '../analyzer/importResult';
 import { isTypedKwargs } from '../analyzer/parameterUtils';
 import * as ParseTreeUtils from '../analyzer/parseTreeUtils';
@@ -346,7 +346,10 @@ export class CompletionProvider {
             return;
         }
 
-        if (completionItemData.modulePath) {
+        if (
+            completionItemData.modulePath &&
+            ImportResolver.isSupportedImportSourceFile(completionItemData.modulePath)
+        ) {
             const documentation = getModuleDocStringFromPaths([completionItemData.modulePath], this.sourceMapper);
             if (!documentation) {
                 return;