Skip to content
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

False-positive void-return in dtslint after adding awaited #37551

Closed
Maxim-Mazurok opened this issue Mar 24, 2020 · 4 comments · Fixed by #37610
Closed

False-positive void-return in dtslint after adding awaited #37551

Maxim-Mazurok opened this issue Mar 24, 2020 · 4 comments · Fixed by #37610
Labels
External Relates to another program, environment, or user action which we cannot control.

Comments

@Maxim-Mazurok
Copy link

TypeScript Version: from 3.9.0-dev.20200321 to 3.9.0-dev.20200324

Code

declare namespace gapi.client {
    function load(name: "abusiveexperiencereport", version: "v1"): PromiseLike<void>;
    function load(name: "abusiveexperiencereport", version: "v1", callback: () => any): void;
}

Expected behavior:
dtslint uses rule void-return which didn't fail for the above code and shouldn't fail.

Actual behavior:
When dtslint is paired with typescript version since this commit: e3ec7b1 which was merged in #35998 - it fails:

ERROR: 17:80  void-return  Use the `void` type for return types only. Otherwise, use `undefined`. See: https://github.com/Microsoft/dtslint/blob/master/docs/void-return.md
ERROR: 18:89  void-return  Use the `void` type for return types only. Otherwise, use `undefined`. See: https://github.com/Microsoft/dtslint/blob/master/docs/void-return.md

Related to Maxim-Mazurok/google-api-typings-generator#101

@Maxim-Mazurok
Copy link
Author

Maxim-Mazurok commented Mar 24, 2020

Sorry, it seems like a dstlint issue actually.
It's using bundled TS (node_modules/dtslint/nodemodules/typescript) for rules, but project's TS (node_modules/typescript) for parsing sources before running rules on them.
So, node kinds reported by older TS version are not matching with the new ones (for example, SyntaxKind.VoidKeyword). That's what was causing problems, as node kinds were updated in this PR.
Probably it would be great to have backward-compatibility in this matter, so I'm not closing the issue.

@RyanCavanaugh RyanCavanaugh added the External Relates to another program, environment, or user action which we cannot control. label Mar 24, 2020
@RyanCavanaugh
Copy link
Member

We don't support this kind of back compat

@Maxim-Mazurok
Copy link
Author

@RyanCavanaugh I hear you. But what if we could support it?
From what I understand, it would require changing

export const enum SyntaxKind {
Unknown,
EndOfFileToken,
SingleLineCommentTrivia,
to

export const enum SyntaxKind { 
     Unknown = "Unknown", 
     EndOfFileToken = "EndOfFileToken", 
     SingleLineCommentTrivia = "SingleLineCommentTrivia",

Kind of like it was did in

export const enum InternalSymbolName {
Call = "__call", // Call signatures
Constructor = "__constructor", // Constructor implementations
New = "__new", // Constructor signatures

Another approach is to do this like for different flags

export const enum CheckFlags {
Instantiated = 1 << 0, // Instantiated symbol
SyntheticProperty = 1 << 1, // Property in union or intersection type
SyntheticMethod = 1 << 2, // Method in union or intersection type
so it will look like

export const enum SyntaxKind { 
     Unknown                 = 1 << 0, 
     EndOfFileToken          = 2 << 0, 
     SingleLineCommentTrivia = 3 << 0,

or even simpler:

export const enum SyntaxKind { 
     Unknown                 = 0, 
     EndOfFileToken          = 1, 
     SingleLineCommentTrivia = 2,

Any of these approaches will preserve enum values and make TS even more back compat :) What do you think?

@Maxim-Mazurok
Copy link
Author

@rbuckton what do you think about #37551 (comment) ?
I think if we use this approach - we can eliminate potential issues when updating SyntaxKind in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Relates to another program, environment, or user action which we cannot control.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants