Disable doc inheritence from builtins, test fixup (#2280)

Co-authored-by: Bill Schnurr <bschnurr@microsoft.com>

packages/pyright-internal/src/analyzer/typeDocStringUtils.ts

@@ -30,6 +30,7 @@ import {
     ModuleType,
     OverloadedFunctionType,
     Type,
+    TypeCategory,
 } from '../analyzer/types';
 import { ModuleNode, ParseNodeType } from '../parser/parseNodes';
 import { TypeEvaluator } from './typeEvaluator';
@@ -42,10 +43,29 @@ import {
 } from './typeUtils';
 
 const DefaultClassIteratorFlagsForFunctions =
+    ClassMemberLookupFlags.SkipObjectBaseClass |
     ClassMemberLookupFlags.SkipInstanceVariables |
     ClassMemberLookupFlags.SkipOriginalClass |
     ClassMemberLookupFlags.DeclaredTypesOnly;
 
+function isInheritedFromBuiltin(type: FunctionType | OverloadedFunctionType, classType?: ClassType): boolean {
+    if (type.category === TypeCategory.OverloadedFunction) {
+        if (type.overloads.length === 0) {
+            return false;
+        }
+        type = type.overloads[0];
+    }
+
+    // Functions that are bound to a different type than where they
+    // were declared are inherited.
+    return (
+        type.details.moduleName === 'builtins' &&
+        !!classType &&
+        !!type.boundToType &&
+        !ClassType.isSameGenericClass(classType, type.boundToType)
+    );
+}
+
 export function getFunctionDocStringInherited(
     type: FunctionType,
     resolvedDecl: Declaration | undefined,
@@ -54,7 +74,10 @@ export function getFunctionDocStringInherited(
 ) {
     let docString: string | undefined;
 
-    if (resolvedDecl && isFunctionDeclaration(resolvedDecl)) {
+    // Don't allow docs to be inherited from the builtins to other classes;
+    // they typically not helpful (and object's __init__ doc causes issues
+    // with our current docstring traversal).
+    if (!isInheritedFromBuiltin(type, classType) && resolvedDecl && isFunctionDeclaration(resolvedDecl)) {
         docString = _getFunctionDocString(type, resolvedDecl, sourceMapper);
     }
 
@@ -87,10 +110,17 @@ export function getOverloadedFunctionDocStringsInherited(
     evaluator: TypeEvaluator,
     classType?: ClassType
 ) {
-    let docStrings = _getOverloadedFunctionDocStrings(type, resolvedDecl, sourceMapper);
+    let docStrings: string[] | undefined;
+
+    // Don't allow docs to be inherited from the builtins to other classes;
+    // they typically not helpful (and object's __init__ doc causes issues
+    // with our current docstring traversal).
+    if (!isInheritedFromBuiltin(type, classType)) {
+        docStrings = _getOverloadedFunctionDocStrings(type, resolvedDecl, sourceMapper);
         if (docStrings && docStrings.length > 0) {
             return docStrings;
         }
+    }
 
     // Search mro
     if (classType && type.overloads.length > 0) {

packages/pyright-internal/src/analyzer/typeEvaluator.ts

@@ -1626,11 +1626,7 @@ export function createTypeEvaluator(
         memberName: string,
         treatConstructorAsClassMember = false
     ): FunctionType | OverloadedFunctionType | undefined {
-        const memberInfo = lookUpClassMember(
+        const memberInfo = lookUpClassMember(classType, memberName, ClassMemberLookupFlags.SkipInstanceVariables);
-            classType,
-            memberName,
-            ClassMemberLookupFlags.SkipInstanceVariables | ClassMemberLookupFlags.SkipObjectBaseClass
-        );
 
         if (memberInfo) {
             const unboundMethodType = getTypeOfMember(memberInfo);

packages/pyright-internal/src/languageService/completionProvider.ts

@@ -59,6 +59,7 @@ import {
     isOverloadedFunction,
     isUnbound,
     isUnknown,
+    OverloadedFunctionType,
     Type,
     TypeBase,
     UnknownType,
@@ -2098,7 +2099,17 @@ export class CompletionProvider {
                             } else if (isInstantiableClass(type)) {
                                 documentation = getClassDocString(type, primaryDecl, this._sourceMapper);
                             } else if (isFunction(type)) {
-                                documentation = getFunctionDocStringFromType(type, this._sourceMapper, this._evaluator);
+                                const functionType = detail.boundObject
+                                    ? (this._evaluator.bindFunctionToClassOrObject(
+                                          detail.boundObject,
+                                          type
+                                      ) as FunctionType)
+                                    : type;
+                                documentation = getFunctionDocStringFromType(
+                                    functionType,
+                                    this._sourceMapper,
+                                    this._evaluator
+                                );
                             } else if (isOverloadedFunction(type)) {
                                 const enclosingClass = isFunctionDeclaration(primaryDecl)
                                     ? ParseTreeUtils.getEnclosingClass(primaryDecl.node.name, false)
@@ -2106,8 +2117,14 @@ export class CompletionProvider {
                                 const classResults = enclosingClass
                                     ? this._evaluator.getTypeOfClass(enclosingClass)
                                     : undefined;
+                                const functionType = detail.boundObject
+                                    ? (this._evaluator.bindFunctionToClassOrObject(
+                                          detail.boundObject,
+                                          type
+                                      ) as OverloadedFunctionType)
+                                    : type;
                                 documentation = getOverloadedFunctionDocStringsInherited(
-                                    type,
+                                    functionType,
                                     primaryDecl,
                                     this._sourceMapper,
                                     this._evaluator,

packages/pyright-internal/src/languageService/hoverProvider.ts

@@ -279,11 +279,7 @@ export class HoverProvider {
             return false;
         }
 
-        const initMethodMember = lookUpClassMember(
+        const initMethodMember = lookUpClassMember(classType, '__init__', ClassMemberLookupFlags.SkipInstanceVariables);
-            classType,
-            '__init__',
-            ClassMemberLookupFlags.SkipInstanceVariables | ClassMemberLookupFlags.SkipObjectBaseClass
-        );
 
         if (!initMethodMember) {
             return false;

packages/pyright-internal/src/tests/fourslash/completions.builtinDocstrings.fourslash.ts

@@ -0,0 +1,117 @@
+/// <reference path="fourslash.ts" />
+
+// @filename: docstrings.py
+//// class A: ...
+////
+//// class B:
+////     """This is the class doc for B."""
+////     def __init__(self):
+////         """This is the __init__ doc for B."""
+////
+//// class C:
+////     """This is the class doc for C."""
+////     def __init__(self):
+////         pass
+////
+//// class D:
+////     def __init__(self):
+////         """This is the __init__ doc for D."""
+////         pass
+////
+//// [|/*global*/|]
+//// object().[|/*object*/|]
+//// A().[|/*a*/|]
+//// B().[|/*b*/|]
+//// C().[|/*c*/|]
+//// D().[|/*d*/|]
+
+// @filename: typeshed-fallback/stdlib/builtins.py
+//// class object():
+////     """This is the class doc for object."""
+////     def __init__(self):
+////         """This is the __init__ doc for object."""
+////         pass
+////
+////     def __dir__(self):
+////         """This is the __dir__ doc for object."""
+////         pass
+
+{
+    // @ts-ignore
+    await helper.verifyCompletion('included', 'plaintext', {
+        global: {
+            completions: [
+                {
+                    label: 'object',
+                    kind: Consts.CompletionItemKind.Class,
+                    documentation: 'class object()\n\nThis is the class doc for object.',
+                },
+                {
+                    label: 'A',
+                    kind: Consts.CompletionItemKind.Class,
+                    documentation: 'class A()',
+                },
+                {
+                    label: 'B',
+                    kind: Consts.CompletionItemKind.Class,
+                    documentation: 'class B()\n\nThis is the class doc for B.',
+                },
+                {
+                    label: 'C',
+                    kind: Consts.CompletionItemKind.Class,
+                    documentation: 'class C()\n\nThis is the class doc for C.',
+                },
+                {
+                    label: 'D',
+                    kind: Consts.CompletionItemKind.Class,
+                    documentation: 'class D()',
+                },
+            ],
+        },
+        object: {
+            completions: [
+                {
+                    label: '__init__',
+                    kind: Consts.CompletionItemKind.Method,
+                    documentation: '__init__: () -> None\n\nThis is the __init__ doc for object.',
+                },
+            ],
+        },
+        a: {
+            completions: [
+                {
+                    label: '__init__',
+                    kind: Consts.CompletionItemKind.Method,
+                    documentation: '__init__: () -> None',
+                },
+            ],
+        },
+        b: {
+            completions: [
+                {
+                    label: '__init__',
+                    kind: Consts.CompletionItemKind.Method,
+                    documentation: '__init__: () -> None\n\nThis is the __init__ doc for B.',
+                },
+            ],
+        },
+        c: {
+            completions: [
+                {
+                    label: '__init__',
+                    kind: Consts.CompletionItemKind.Method,
+                    documentation: '__init__: () -> None',
+                },
+            ],
+        },
+        d: {
+            completions: [
+                {
+                    label: '__init__',
+                    kind: Consts.CompletionItemKind.Method,
+                    documentation: '__init__: () -> None\n\nThis is the __init__ doc for D.',
+                },
+            ],
+        },
+    });
+}

packages/pyright-internal/src/tests/fourslash/completions.importSubmodule.fourslash.ts

@@ -19,7 +19,7 @@
 ////
 
 // @ts-ignore
-await helper.verifyCompletion('included', 'markdown', {
+await helper.verifyCompletion('exact', 'markdown', {
     marker1: {
         completions: [
             { label: 'setup', kind: Consts.CompletionItemKind.Module },

packages/pyright-internal/src/tests/fourslash/hover.builtinDocstrings.fourslash.ts

@@ -0,0 +1,64 @@
+/// <reference path="fourslash.ts" />
+
+// @filename: docstrings.py
+//// [|/*object*/object|]
+//// [|/*objectInit*/object|]()
+//// object().[|/*objectDir*/__dir__|]
+////
+//// class A: ...
+////
+//// [|/*a*/A|]
+//// [|/*aInit*/A|]()
+//// A().[|/*aDir*/__dir__|]
+////
+//// class B:
+////     """This is the class doc for B."""
+////     def __init__(self):
+////         """This is the __init__ doc for B."""
+////
+//// [|/*b*/B|]
+//// [|/*bInit*/B|]()
+////
+//// class C:
+////     """This is the class doc for C."""
+////     def __init__(self):
+////         pass
+////
+//// [|/*c*/C|]
+//// [|/*cInit*/C|]()
+////
+//// class D:
+////     def __init__(self):
+////         """This is the __init__ doc for D."""
+////         pass
+////
+//// [|/*d*/D|]
+//// [|/*dInit*/D|]()
+
+// @filename: typeshed-fallback/stdlib/builtins.py
+//// class object():
+////     """This is the class doc for object."""
+////     def __init__(self):
+////         """This is the __init__ doc for object."""
+////         pass
+////
+////     def __dir__(self):
+////         """This is the __dir__ doc for object."""
+////         pass
+
+{
+    helper.verifyHover('plaintext', {
+        object: '(class) object\n\nThis is the class doc for object.',
+        objectInit: '(class) object()\n\nThis is the __init__ doc for object.',
+        objectDir: '(method) __dir__: () -> Iterable[str]\n\nThis is the __dir__ doc for object.',
+        a: '(class) A',
+        aInit: '(class) A()',
+        aDir: '(method) __dir__: () -> Iterable[str]',
+        b: '(class) B\n\nThis is the class doc for B.',
+        bInit: '(class) B()\n\nThis is the __init__ doc for B.',
+        c: '(class) C\n\nThis is the class doc for C.',
+        cInit: '(class) C()\n\nThis is the class doc for C.',
+        d: '(class) D',
+        dInit: '(class) D()\n\nThis is the __init__ doc for D.',
+    });
+}

packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.fourslash.ts

@@ -71,9 +71,9 @@
 //// print(inner.[|/*inner_method1_docs*/method1|]())
 
 helper.verifyHover('markdown', {
-    a_docs: '```python\n(class) A\n```\n---\nA docs',
+    a_docs: '```python\n(class) A()\n```\n---\nA docs',
     b_docs: '```python\n(class) B()\n```\n---\nB init docs',
-    a_inner_docs: '```python\n(class) Inner\n```\n---\nA.Inner docs',
+    a_inner_docs: '```python\n(class) Inner()\n```\n---\nA.Inner docs',
     func1_docs: '```python\n(function) func1: () -> bool\n```\n---\nfunc1 docs',
     func2_docs: '```python\n(function) func2: () -> bool\n```\n---\nfunc2 docs',
     inner_method1_docs: '```python\n(method) method1: () -> bool\n```\n---\nA.Inner.method1 docs',

packages/pyright-internal/src/tests/fourslash/hover.inherited.docFromSrc.fourslash.ts

@@ -55,9 +55,9 @@
 
 helper.verifyHover('markdown', {
     child_a_method1_docs: '```python\n(method) method1: () -> bool\n```\n---\nA.method1 docs',
-    child_a_docs: '```python\n(class) ChildA\n```',
+    child_a_docs: '```python\n(class) ChildA()\n```',
     child_b_docs: '```python\n(class) ChildB()\n```\n---\nB init docs',
     child_b_init_docs: '```python\n(method) __init__: () -> None\n```\n---\nB init docs',
-    secondDerived_docs: '```python\n(class) Derived2\n```',
+    secondDerived_docs: '```python\n(class) Derived2()\n```',
     secondDerived_method_docs: '```python\n(method) method: () -> None\n```\n---\nBase.method docs',
 });

packages/pyright-internal/src/tests/fourslash/hover.inherited.docFromSrcWithStub.fourslash.ts

@@ -47,8 +47,8 @@
 
 helper.verifyHover('markdown', {
     child_a_method1_docs: '```python\n(method) method1: () -> bool\n```\n---\nA.method1 docs',
-    child_a_docs: '```python\n(class) ChildA\n```',
+    child_a_docs: '```python\n(class) ChildA()\n```',
-    child_a_inner_docs: '```python\n(class) ChildInner\n```',
+    child_a_inner_docs: '```python\n(class) ChildInner()\n```',
     child_a_inner_method1_docs: '```python\n(method) method1: () -> bool\n```\n---\nA.Inner.method1 docs',
-    child_b_docs: '```python\n(class) ChildB\n```',
+    child_b_docs: '```python\n(class) ChildB()\n```',
 });

packages/pyright-internal/src/tests/fourslash/hover.inherited.docFromStub.fourslash.ts

@@ -36,7 +36,7 @@
 
 helper.verifyHover('markdown', {
     child_a_method1_docs: '```python\n(method) method1: () -> bool\n```\n---\nA.method1 docs',
-    child_a_docs: '```python\n(class) ChildA\n```',
+    child_a_docs: '```python\n(class) ChildA()\n```',
-    child_a_inner_docs: '```python\n(class) ChildInner\n```',
+    child_a_inner_docs: '```python\n(class) ChildInner()\n```',
     child_a_inner_method1_docs: '```python\n(method) method1: () -> bool\n```\n---\nA.Inner.method1 docs',
 });

packages/pyright-internal/src/tests/fourslash/hover.libCodeAndStub.fourslash.ts

@@ -44,7 +44,7 @@
 //// obj.[|/*marker5*/read_write_prop|] = r
 
 helper.verifyHover('markdown', {
-    marker1: '```python\n(class) Validator\n```\n---\nThe validator class',
+    marker1: '```python\n(class) Validator()\n```\n---\nThe validator class',
     marker2: '```python\n(method) is_valid: (text: str) -> bool\n```\n---\nChecks if the input string is valid.',
     marker3: '```python\n(property) read_only_prop: bool\n```\n---\nThe read-only property.',
     marker4: '```python\n(property) read_write_prop: bool\n```\n---\nThe read-write property.',

packages/pyright-internal/src/tests/fourslash/hover.libCodeNoStub.fourslash.ts

@@ -33,7 +33,7 @@
 //// obj.[|/*marker5*/read_write_prop|] = r
 
 helper.verifyHover('markdown', {
-    marker1: '```python\n(class) Validator\n```\n---\nThe validator class',
+    marker1: '```python\n(class) Validator()\n```\n---\nThe validator class',
     marker2: '```python\n(method) is_valid: (text: str) -> bool\n```\n---\nChecks if the input string is valid.',
     marker3: '```python\n(property) read_only_prop: bool\n```\n---\nThe read-only property.',
     marker4: '```python\n(property) read_write_prop: bool\n```\n---\nThe read-write property.',

packages/pyright-internal/src/tests/fourslash/hover.libStub.fourslash.ts

@@ -33,7 +33,7 @@
 //// obj.[|/*marker5*/read_write_prop|] = r
 
 helper.verifyHover('markdown', {
-    marker1: '```python\n(class) Validator\n```\n---\nThe validator class',
+    marker1: '```python\n(class) Validator()\n```\n---\nThe validator class',
     marker2: '```python\n(method) is_valid: (text: str) -> bool\n```\n---\nChecks if the input string is valid.',
     marker3: '```python\n(property) read_only_prop: bool\n```\n---\nThe read-only property.',
     marker4: '```python\n(property) read_write_prop: bool\n```\n---\nThe read-write property.',

packages/pyright-internal/src/tests/fourslash/signature.builtinDocstrings.fourslash.ts

@@ -0,0 +1,63 @@
+/// <reference path="fourslash.ts" />
+
+// @filename: docstrings.py
+//// class A: ...
+////
+//// class B:
+////     """This is the class doc for B."""
+////     def __init__(self):
+////         """This is the __init__ doc for B."""
+////
+//// class C:
+////     """This is the class doc for C."""
+////     def __init__(self):
+////         pass
+////
+//// class D:
+////     def __init__(self):
+////         """This is the __init__ doc for D."""
+////         pass
+////
+//// object([|/*object*/|])
+//// A([|/*a*/|])
+//// B([|/*b*/|])
+//// C([|/*c*/|])
+//// D([|/*d*/|])
+
+// @filename: typeshed-fallback/stdlib/builtins.py
+//// class object():
+////     """This is the class doc for object."""
+////     def __init__(self):
+////         """This is the __init__ doc for object."""
+////         pass
+////
+////     def __dir__(self):
+////         """This is the __dir__ doc for object."""
+////         pass
+
+{
+    helper.verifySignature('plaintext', {
+        object: {
+            signatures: [
+                { label: '() -> None', parameters: [], documentation: 'This is the __init__ doc for object.' },
+            ],
+            activeParameters: [undefined],
+        },
+        a: {
+            signatures: [{ label: '() -> None', parameters: [] }],
+            activeParameters: [undefined],
+        },
+        b: {
+            signatures: [{ label: '() -> None', parameters: [], documentation: 'This is the __init__ doc for B.' }],
+            activeParameters: [undefined],
+        },
+        c: {
+            signatures: [{ label: '() -> None', parameters: [], documentation: 'This is the class doc for C.' }],
+            activeParameters: [undefined],
+        },
+        d: {
+            signatures: [{ label: '() -> None', parameters: [], documentation: 'This is the __init__ doc for D.' }],
+            activeParameters: [undefined],
+        },
+    });
+}