Skip to content

Assorted find-all-references bugs #13766

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

Closed
ghost opened this issue Jan 30, 2017 · 1 comment
Closed

Assorted find-all-references bugs #13766

ghost opened this issue Jan 30, 2017 · 1 comment
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@ghost
Copy link

ghost commented Jan 30, 2017

I made a list of things I discovered while working on #13760.

  • Some tests don't have isWriteAccess when they should

    • findAllRefsParameterPropertyDeclaration1 (/2, /3): this.privateParam += 10 should be a write access
    • referencesForNumericLiteralPropertyNames: These should be write accesses if normally-named properties are
    • referencesForStringLiteralPropertyNames (/4): Same as above
    • referencesForClassParameter: All should be write accesses
    • localGetReferences (e.g. fooCls.clsSVar++)
    • getOccurrencesIsDefinitionOfComputedProperty: definition should be a write access
    • referenceInParameterPropertyDeclaration
    • remoteGetReferences
  • Some tests have multiple groups with identical definitions.

    • findAllRefsForObjectLiteralProperties
    • findAllRefsOnDefinition: r1
    • findAllRefsWithLeadingUnderscoreNames1 (/2, /3, /4)
    • findAllRefsWithShorthandPropertyAssignment: r4
    • findAllRefsWithShorthandPropertyAssignment2: r3
    • referencesForInheritedProperties: r3
    • referencesForInheritedProperties2: r3
    • referencesForInheritedProperties4: r1
    • referencesForObjectLiteralProperties
    • referencesForPropertiesOfGenericTypes
    • ambientShorthandFindAllRefs
    • getReferencesAtPosition
    • getOccurrencesIsDefinitionOfStringNamedProperty
    • referencesForClassParameter
    • referencesForClassLocal
    • referencesBloomFilters (/2)
    • getOccurrencesIsDefinitionOfNumberNamedProperty
    • cancellationWhenFindingAllRefsOnDefinition
    • referencesForClassMembers: m2
  • Some tests have a different ordering of merged declarations depending on where you start.
    (Maybe this is fine.)

    • findAllRefsForDefaultExport03
    • findAllRefsInheritedProperties1
    • findAllRefsInheritedProperties3 (see r6)
    • referencesBloomFilters (/2)
  • Other

    • findAllRefsForComputedProperties: I[["prop1"]] should be just I["prop1"].
    • findAllRefsForMappedType:
      It's iffy that (property) a: string only shows up when you're directly using it, and otherwise is grouped under (property) T.a: number.
    • findAllRefsObjectBindingElementPropertyName06: r4 could be just one definition
    • findAllRefsObjectBindingElementPropertyName07:
      The first a shouldn't be a write access (or a definition): it's just the property name we're destructuring from. p is the definition.
    • findAllRefsOnImportAliases2: In import Class as C2, Class doesn't really deserve "isDefinition"
    • findAllRefsThisKeyword:
      Wierd that "f0" and "f1" have different definitions. Same for "propDef" and "propUse".
    • findAllRefsForObjectSpread:
      A2.a: number should be A2.a?: number
    • referencesForContextuallyTypedUnionProperties (/2):
      Strange how most of them are considered A.common.
    • referencesForEnums:
      If value1 is a write access, value2 should be too
    • referencesForMergedDeclarations: two of the definitions do not contain "interface Foo", why?
    • renameDefaultImportDifferentName:
      "constructor C(): B" is iffy, since 'C' and 'B' are the same thing
    • getOccurrencesIsDefinitionOfComputedProperty: Should work for r1 and r2
    • referencesForClassMembersExtendingGenericClass:
      Wierd that for m1, it shows as Base<T>.method(): void
@ghost
Copy link
Author

ghost commented Sep 7, 2018

These are all fixed now. 🎊

@ghost ghost closed this as completed Sep 7, 2018
@ghost ghost added the Fixed A PR has been merged for this issue label Sep 7, 2018
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

1 participant