Skip to content

Fixes #30507 #32100

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
timsuchanek wants to merge 15 commits into from
Closed

Fixes #30507 #32100

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
8e37e0a
fixes #30507
timsuchanek Jun 25, 2019
5c561ea
Add test case for generic Partial type
timsuchanek Jul 12, 2019
d674ee0
Fixes #28470
timsuchanek Jul 12, 2019
ad166f7
Simplify contextFlags binary check
timsuchanek Jul 16, 2019
50286b9
Add string literal completion test
timsuchanek Jul 16, 2019
ba1e478
Fix ContextFlags typings
timsuchanek Aug 14, 2019
adf8a54
Speed up inference expression for completion
timsuchanek Aug 14, 2019
25fa027
Fix baseline merge
andrewbranch Sep 4, 2019
7883ec6
Make contextFlags internal
timsuchanek Sep 11, 2019
7e7dc08
Reapply readonly array changes
timsuchanek Sep 11, 2019
c1a47a5
accept baselines
timsuchanek Sep 11, 2019
96a782e
Fix generic completion tests
timsuchanek Sep 11, 2019
5ae31ff
Merge master
orta Sep 25, 2019
b863e01
Merge branch 'master' into genericObjectExpressionCompletion
andrewbranch Oct 10, 2019
d1cb5cb
Re-merge ContextFlags
andrewbranch Oct 10, 2019
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
Prev Previous commit
Next Next commit
Fix baseline merge
  • Loading branch information
@andrewbranch @timsuchanek
andrewbranch authored and timsuchanek committed Sep 11, 2019
commit 25fa027721d8f3e7451119f0c9ca8ef5d59301cc
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2026,7 +2026,7 @@ declare namespace ts {
*/
runWithCancellationToken<T>(token: CancellationToken, cb: (checker: TypeChecker) => T): T;
}
enum ContextFlags {
export enum ContextFlags {
None = 0,
Signature = 1,
Completion = 2
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/api/typescript.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2026,12 +2026,12 @@ declare namespace ts {
*/
runWithCancellationToken<T>(token: CancellationToken, cb: (checker: TypeChecker) => T): T;
}
enum ContextFlags {
export enum ContextFlags {
Copy link
Member

@sheetalkamat sheetalkamat Sep 5, 2019

Choose a reason for hiding this comment

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

I think this as well as new signature need to be internal

Copy link
Contributor Author

@timsuchanek timsuchanek Sep 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Do you mean we should use something different than the ContextFlags when calling the code from the completion service to prevent this type being exposed?

Copy link
Member

@andrewbranch andrewbranch Sep 10, 2019

Choose a reason for hiding this comment

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

I think Sheetal means to move ContextFlags into one of the ts namespaces that’s already marked /** @internal */ and then to mark the second parameter of TypeChecker['getContextualType'] as /* @internal */ (or declare it as an internal overload, but the linter might call that an unnecessary overload)

Copy link
Contributor Author

@timsuchanek timsuchanek Sep 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Ok, makes sense! The overload option indeed makes the linter complain:

ERROR: /Users/tim/code/TypeScript/src/compiler/types.ts:3224:61 unified-signatures These overloads can be combined into one signature with an optional parameter.

The non-overload option, which I wrote like this:

        getContextualType(
            node: Expression,
            /* @internal */ contextFlags?: ContextFlags
        ): Type | undefined;

spits out an invalid typescript.d.ts:

        getRootSymbols(symbol: Symbol): ReadonlyArray<Symbol>;
        getContextualType(node: Expression, 
: Type | undefined;
        /**
         * returns unknownSignature in the case of an error.
         * returns undefined if the node is not valid.
         * @param argumentCount Apparent number of arguments, passed in case of a possibly incomplete call. This should come from an ArgumentListInfo. See `signatureHelp.ts`.
         */

Is there a way to ignore the linter in this case?
Or what is the right way to add @internal to a specific argument?

Copy link
Member

@andrewbranch andrewbranch Sep 10, 2019

Choose a reason for hiding this comment

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

Just filed #33350. I think it would be ok to ignore the line from the linter. @sheetalkamat?

Copy link
Member

@sheetalkamat sheetalkamat Sep 10, 2019

Choose a reason for hiding this comment

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

Yes in past we have marked the overload internal and disabled the rule for that line.

Copy link
Contributor Author

@timsuchanek timsuchanek Sep 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Done. Had to adjust the tests because of #32266

Anything else you need from my side to get this merged?
@sheetalkamat @andrewbranch

Copy link
Member

@andrewbranch andrewbranch Sep 13, 2019

Choose a reason for hiding this comment

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

Cool, this doesn’t even touch the public API baselines now. Would be good to get an approval from another person on the team but I think this is good.

None = 0,
Signature = 1,
Completion = 2
}
enum NodeBuilderFlags {
export enum NodeBuilderFlags {
None = 0,
NoTruncation = 1,
WriteArrayAsGenericType = 2,
Expand Down