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.

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

Jump to bottom

URI.revive is not well typed #71992

Closed
alexdima opened this issue Apr 9, 2019 · 3 comments
Closed

URI.revive is not well typed #71992

alexdima opened this issue Apr 9, 2019 · 3 comments
Assignees
@jrieken
Labels
bug Issue identified by VS Code Team member as probable bug debt Code quality issues uri verified Verification succeeded
Milestone
April 2019

Comments

@alexdima
Copy link
Member

alexdima commented Apr 9, 2019

Today it is:

	static revive(data: UriComponents | any): URI {
        ...
    }

It should be something like:

    static revive(data: UriComponents): URI;
    static revive(data: UriComponents | undefined): URI | undefined;
    static revive(data: UriComponents | null): URI | null;
    static revive(data: UriComponents | null | undefined): URI | null | undefined;
    static revive(data: UriComponents | null | undefined): URI | null | undefined {
        ...
    }

cc @weinand
@jrieken jrieken added debt Code quality issues uri labels Apr 9, 2019
@jrieken jrieken added this to the April 2019 milestone Apr 9, 2019
@weinand
Copy link
Contributor

weinand commented Apr 9, 2019

I was running into this issue when I changed some type from being a URL to URL[] and the "revive" would happily accept it...

@jrieken
Copy link
Member

jrieken commented Apr 12, 2019

Fixed.

@aeschli This is the only thing that now looks fishy: https://github.com/Microsoft/vscode/blob/261fa9f93910ec25a9836fcfe68d7f3cb4a6a06b/src/vs/workbench/electron-browser/window.ts#L529

jrieken added a commit that referenced this issue Apr 12, 2019
@jrieken
also make null-lovers happy, #71992
5ccbc56
@jrieken jrieken added the bug Issue identified by VS Code Team member as probable bug label Apr 29, 2019
@Tyriar
Copy link
Member

Tyriar commented May 7, 2019

Verified via review. @jrieken FYI you can avoid these casts:

vscode/src/vs/workbench/api/browser/mainThreadSearch.ts

Lines 157 to 168 in 8ef8aa6

dataOrUri.forEach(result => {
if ((<IRawFileMatch2>result).results) {
searchOp.addMatch({
resource: URI.revive((<IRawFileMatch2>result).resource),
results: (<IRawFileMatch2>result).results
});
} else {
searchOp.addMatch({
resource: URI.revive(<UriComponents>result)
});
}
});

Like this:

		dataOrUri.forEach(result => {
			if ('resource' in result) {
				searchOp.addMatch({
					resource: URI.revive(result.resource),
					results: result.results
				});
			} else {
				searchOp.addMatch({
					resource: URI.revive(result)
				});
			}
		});

@Tyriar Tyriar added the verified Verification succeeded label May 7, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators May 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jrieken jrieken

Labels
bug Issue identified by VS Code Team member as probable bug debt Code quality issues uri verified Verification succeeded
Projects
None yet
Milestone
April 2019
Development

No branches or pull requests
4 participants
@jrieken @weinand @Tyriar @alexdima