Change typing hint provision

njqdev · njqdev · commit 0329b9156ce1 · 2020-04-18T20:50:39.000+02:00
diff --git a/src/completionProvider.ts b/src/completionProvider.ts
@@ -17,18 +17,19 @@ import { TypeHintSettings } from "./settings";
 
 export abstract class CompletionProvider {
 
-    protected pushTypesToItems(typeNames: PythonType[], completionItems: CompletionItem[]) {
-        for (const typeName of typeNames) {
-            const item = new CompletionItem(this.labelFor(typeName), CompletionItemKind.TypeParameter);
-    
-            // Add 999 to ensure they're sorted to the bottom of the CompletionList.
-            item.sortText = `999${typeName}`;
+    protected bottomOfListSortPrefix: number = 999;
+
+    protected pushHintsToItems(typeHints: string[], completionItems: CompletionItem[]) {
+        const sortTextPrefix = this.bottomOfListSortPrefix.toString();
+        for (const hint of typeHints) {
+            const item = new CompletionItem(this.labelFor(hint), CompletionItemKind.TypeParameter);
+            item.sortText = sortTextPrefix + hint;
             completionItems.push(item);
         }
     }
 
-    protected labelFor(typeName: string): string {
-        return " " + typeName;
+    protected labelFor(typeHint: string): string {
+        return " " + typeHint;
     }
 
     abstract async provideCompletionItems(
@@ -69,9 +70,20 @@ export class ParamHintCompletionProvider extends CompletionProvider implements C
                 if (param) {
                     try {
                         this.pushEstimationsToItems(await provider.getTypeHints(param), items); 
-                    } catch (error) {
-                    }   
-                    this.pushTypesToItems(provider.getRemainingTypes(), items);
+                    } catch {
+                    }
+
+                    const sortTextPrefix = (this.bottomOfListSortPrefix - 1).toString();
+                    for (const hint of provider.remainingTypeHints()) {
+                        let item = new CompletionItem(this.labelFor(hint), CompletionItemKind.TypeParameter);
+                        item.sortText = sortTextPrefix + hint;
+                        items.push(item);
+                    }
+                    
+                    if (provider.typingImported) {
+                        this.pushHintsToItems(provider.remainingTypingHints(), items);
+                    }
+                    
                     return Promise.resolve(new CompletionList(items, false));  
                 }
             }
@@ -85,16 +97,8 @@ export class ParamHintCompletionProvider extends CompletionProvider implements C
      * @param precedingText The text before the active position.
      */
     private getParam(precedingText: string): string | null {
-        let param = "";
-
-        let i = precedingText.length - 1;
-        let last = precedingText[i];
-        while (last !== "," && last !== "(" && last !== "*" && i >= 0) {
-            param = precedingText[i] + param;
-            i--;
-            last = precedingText[i];
-        }
-        param = param.trim();
+        const split = precedingText.split(/[,(*]/);
+        let param = split.length > 1 ? split[split.length - 1].trim() : precedingText;
         return !param || /[!:?/\\{}.+/=)'";@&£%¤|<>$^~¨ -]/.test(param) ? null : param;
     }
     
@@ -117,7 +121,7 @@ export class ParamHintCompletionProvider extends CompletionProvider implements C
     private shouldProvideItems(precedingText: string, activePos: Position, doc: TextDocument): boolean {
 
         if (activePos.character > 0 && !/#/.test(precedingText)) {
-            let provide = new RegExp("^[ \t]*def [^(]+\\(", "m").test(precedingText);
+            let provide = /^[ \t]*def /.test(precedingText);
 
             if (!provide) {
                 const nLinesToCheck = activePos.line > 4 ? 4 : activePos.line;
@@ -150,7 +154,7 @@ export class ReturnHintCompletionProvider extends CompletionProvider implements
         const line = doc.lineAt(pos);
 
         if (this.shouldProvideItems(line, pos)) {         
-            this.pushTypesToItems(Object.values(PythonType), items);
+            this.pushHintsToItems(Object.values(PythonType), items);
         }
         return Promise.resolve(new CompletionList(items, false));
     }
diff --git a/src/typeHintProvider.ts b/src/typeHintProvider.ts
@@ -10,6 +10,9 @@ import { TypeHintSettings } from "./settings";
  */
 export class TypeHintProvider {
     
+    private doc: TextDocument;
+    private settings: TypeHintSettings;
+    private typingHintProvider: TypingHintProvider;
     private likelyTypes: PythonType[] = [
         PythonType.String,
         PythonType.List, 
@@ -19,10 +22,9 @@ export class TypeHintProvider {
         PythonType.Tuple, 
         PythonType.Float
     ];
+    private providedTypeHints: string[] = [];
     private typeContainer: DataTypeContainer = getDataTypeContainer();
-    private typeNamesIncludedInResult: string[] = [];
-    private doc: TextDocument;
-    private settings: TypeHintSettings;
+    private _typingImported = false;
 
     /**
      * Constructs a new TypeHintProvider.
@@ -33,20 +35,24 @@ export class TypeHintProvider {
     constructor(doc: TextDocument, settings: TypeHintSettings) {
         this.doc = doc;
         this.settings = settings;
+        this.typingHintProvider = new TypingHintProvider(this.typeContainer);
+    }
+
+    public get typingImported() {
+        return this._typingImported;
     }
 
     /**
-     * Provides type hints for a parameter.
+     * Estimates a parameter's type and returns type hints for it. 
+     * The returned hints are ordered with the most likely type being first. 
      * 
      * @param param The parameter name.
-     * @returns An array of type hints, ordered by estimation accuracy.
      */
     public async getTypeHints(param: string): Promise<string[]> {
         const typeHints: string[] = [];
         const documentText = this.doc.getText();
 
-        const typingHintProvider = new TypingHintProvider(documentText, this.typeContainer);
-        const typingSearch = typingHintProvider.containsTyping();
+        const typingSearch = this.typingHintProvider.detectTypingImport(documentText);
 
         const variableSearch = TypeSearch.variableWithSameName(param, documentText);
 
@@ -59,7 +65,7 @@ export class TypeHintProvider {
         let typeName = this.getTypeParamEndsWith(param, "_");
         if (typeName) {
             this.add(typeName, typeHints);
-            this.tryAddTypingHint(await typingSearch, typeName, typingHintProvider, typeHints);
+            this.tryAddTypingHint(await typingSearch, typeName, typeHints);
             wsSearcher.cancel();
             await workspaceSearch;
             return typeHints;
@@ -69,35 +75,44 @@ export class TypeHintProvider {
 
         if (searchResult !== null && !TypeSearch.invalidTernaryOperator(searchResult)) {
             this.add(searchResult.typeName, typeHints);
-            this.tryAddTypingHints(await typingSearch, searchResult, typingHintProvider, typeHints);
+            this.tryAddTypingHints(await typingSearch, searchResult, typeHints);
             this.tryAdd(this.typeGuessFor(param, typeHints), typeHints);
-        } else if (typeName = this.getTypeParamEndsWith(param, "")) {
-            this.add(typeName, typeHints);
-            this.tryAddTypingHint(await typingSearch, typeName, typingHintProvider, typeHints);
         } else {
-            this.tryAdd(this.typeGuessFor(param, typeHints), typeHints);
-        }
+            typeName = this.getTypeParamEndsWith(param, "");
+            if (typeName) {
+                this.add(typeName, typeHints);
+                this.tryAddTypingHint(await typingSearch, typeName, typeHints);
+            } else {
+                this.tryAdd(this.typeGuessFor(param, typeHints), typeHints);
+            }
+        } 
 
         this.tryAdd(await workspaceSearch, typeHints);
         return typeHints;
     }
     
     /**
-     * Gets types that have not been provided as hints.
-     * 
-     * @returns An array of types.
+     * Returns hints for types that have not been provided yet.
      */
-    public getRemainingTypes(): PythonType[] {
+    public remainingTypeHints(): PythonType[] {
         const result: PythonType[] = [];
 
         for (const type of Object.values(this.typeContainer)) {
-            if (this.typeNotIncluded(type.name)) {
+            if (this.hintNotProvided(type.name)) {
                 result.push(type.name);
             }
         }
+
         return result;
     }
 
+    /**
+     * Returns hints for the typing module that have not been provided yet.
+     */
+    public remainingTypingHints(): string[] {
+        return this.typingHintProvider.getRemainingHints();
+    }
+
     /**
      * If the param ends with a type name, the type is returned.
      * @param param The parameter name.
@@ -134,16 +149,16 @@ export class TypeHintProvider {
     }
 
 
-    private add(typeName: string, typeHints: string[]) {
-        typeName = typeName.trim();
-        if (this.typeNotIncluded(typeName)) {
-            typeHints.push(typeName);
-            this.typeNamesIncludedInResult.push(typeName);
+    private add(type: string, typeHints: string[]) {
+        type = type.trim();
+        if (this.hintNotProvided(type)) {
+            typeHints.push(type);
+            this.providedTypeHints.push(type);
         }
     }
             
-    private typeNotIncluded(type: string): boolean {
-        return !this.typeNamesIncludedInResult.includes(type);
+    private hintNotProvided(type: string): boolean {
+        return !this.providedTypeHints.includes(type);
     }
 
     private tryAdd(type: string | null, typeHints: string[]) {
@@ -155,11 +170,10 @@ export class TypeHintProvider {
     private tryAddTypingHints(
         typingFound: boolean,
         searchResult: VariableSearchResult | null,
-        typingHintProvider: TypingHintProvider,
         typeHints: string[]
     ) {
         if (typingFound) {
-            const hints: string[] | null = typingHintProvider.getTypingHints(searchResult);
+            const hints: string[] | null = this.typingHintProvider.getHints(searchResult);
             if (hints) {
                 for (const hint of hints) {
                     this.add(hint, typeHints);
@@ -168,14 +182,10 @@ export class TypeHintProvider {
         }
     }
 
-    private tryAddTypingHint(
-        typingFound: boolean,
-        typeName: string,
-        typingHintProvider: TypingHintProvider,
-        typeHints: string[]
-    ) {
-        if (typingFound) {
-            const typingHint = typingHintProvider.getTypingHint(typeName);
+    private tryAddTypingHint(typingImported: boolean, typeName: string, typeHints: string[]) {
+        this._typingImported = typingImported;
+        if (typingImported) {
+            const typingHint = this.typingHintProvider.getHint(typeName);
             if (typingHint) {
                 this.add(typingHint, typeHints);
             }
diff --git a/src/typeSearch.ts b/src/typeSearch.ts
@@ -96,8 +96,8 @@ export class TypeSearch {
 
         // Searching the import source document is not supported (yet?)
         if (!this.isImported(match[1], src.substr(match.index - match.length))) {
-            
-            if (match = this.variableSearchRegExp(match[1]).exec(src)) {
+            match = this.variableSearchRegExp(match[1]).exec(src);
+            if (match) {
                 const otherType = this.detectType(match[1]);
                 return otherType 
                     ? new VariableSearchResult(otherType, EstimationSource.ValueOfOtherVariable, valueAssignment)
diff --git a/src/typingHintProvider.ts b/src/typingHintProvider.ts
@@ -1,4 +1,4 @@
-import { TypeCategory, PythonType, DataTypeContainer } from "./python";
+import { TypeCategory, PythonType, DataTypeContainer, DataType } from "./python";
 import { capitalized } from "./utils";
 import { VariableSearchResult, TypeSearch } from "./typeSearch";
 
@@ -7,32 +7,31 @@ import { VariableSearchResult, TypeSearch } from "./typeSearch";
  */
 export class TypingHintProvider {
 
-    private docText: string;
     private typeContainer: DataTypeContainer;
     private fromTypingImport: boolean = false;
+    private providedTypes: DataType[] = [];
     private typingPrefix: string = "typing";
 
     /**
      * Constructs a new TypingHintProvider.
      * 
-     * @param docText The document text to search.
      * @param typeContainer A container with built-in Python types.
      */
-    constructor(docText: string, typeContainer: DataTypeContainer) {
-        this.docText = docText;
+    constructor(typeContainer: DataTypeContainer) {
         this.typeContainer = typeContainer;
     }
 
     /**
-     * Determines if this object's document contains a typing import.
+     * Determines if a document contains a typing import.
      * 
+     * @param docText The document text to search.
      * @returns True if typing is imported.
      */
-    public async containsTyping(): Promise<boolean> {
+    public async detectTypingImport(docText: string): Promise<boolean> {
         let m = new RegExp(
             `^[ \t]*from typing import +([A-Z][a-zA-Z0-9 ,]+)`,
             "m"
-        ).exec(this.docText);
+        ).exec(docText);
         
         if (m) {
             this.fromTypingImport = true;
@@ -41,7 +40,7 @@ export class TypingHintProvider {
             m = new RegExp(
                 `^[ \t]*(import +typing +as +([a-zA-Z_][a-zA-Z0-9_-]*)|import +typing)`,
                 "m"
-            ).exec(this.docText);
+            ).exec(docText);
             if (m) {
                 if (m[2]) {
                     this.typingPrefix = m[2];
@@ -58,9 +57,10 @@ export class TypingHintProvider {
      * @param type A type name.
      * @returns A type hint without a closing bracket, for example 'List[ '.
      */
-    public getTypingHint(typeName: string): string | null {
+    public getHint(typeName: string): string | null {
         const type = this.typeContainer[typeName];
         if (type.category === TypeCategory.Collection) {
+            this.providedTypes.push(type);
             return this.toTypingString(type.name);
         }
         return null;
@@ -72,9 +72,12 @@ export class TypingHintProvider {
      * @param searchResult A search result to derive hints from.
      * @returns One or two type hints. For example, 'List[' and 'List[str]'.
      */
-    public getTypingHints(searchResult: VariableSearchResult | null): string[] | null {
+    public getHints(searchResult: VariableSearchResult | null): string[] | null {
+
         if (searchResult && searchResult.typeName in this.typeContainer) {
             const type = this.typeContainer[searchResult.typeName];
+            this.providedTypes.push(type);
+
             const result: string[] = [ this.toTypingString(type.name)];
             let label = result[0];
 
@@ -120,6 +123,22 @@ export class TypingHintProvider {
         return null;
     }
 
+    /**
+     * Get hints for collection types that have not been provided yet.
+     * 
+     * @returns An array of types.
+     */
+    public getRemainingHints(): string[] {
+        const result: string[] = [];
+
+        for (const type of Object.values(this.typeContainer).filter(t => t.category === TypeCategory.Collection)) {
+            if (!this.providedTypes.includes(type)) {
+                result.push(this.toTypingString(type.name));
+            }
+        }
+        return result;
+    }
+
     private toTypingString(typeName: string): string {
         const typingName = capitalized(typeName);
         return this.fromTypingImport ? `${typingName}[` : `${this.typingPrefix}.${typingName}[`;
diff --git a/test/providers/typingHintProvider.test.ts b/test/providers/typingHintProvider.test.ts
