lsp: Quick fix for redirected imports always adds double quotes

[relrelb]

Steps to reproduce behavior

Consider the following code:

// @deno-types='https://esm.sh/@types/node'
import 'https://esm.sh/colors';

Deno's LSP suggests (justifiably) that both URLs are redirected, and provides a quick fix to "update specifier to its redirected specifier".

However, when applying the fixes, the following code is received:

// @deno-types='"https://esm.sh/v106/@types/[email protected]/index.d.ts"'
import "https://esm.sh/[email protected]";

Which is invalid, due to the two pairs of quotes enclosing the @deno-types URL. Also note that the import statement changed from single quotes to double quotes.

A similar error happens with triple-slash reference types directive:

/// <reference types=""https://esm.sh/v106/@types/[email protected]/index.d.ts"" />
import "https://esm.sh/colors";

Expected behavior

Applying the quick fix doesn't add an extra ", and the original quotes are preserved.

Version

Deno v1.30.0

Suggested fix

It seems like the extra " are added here:

https://github.com/denoland/deno/blob/a635c9700c0b99c3ad128c21727fdcfb23bad72f/cli/lsp/diagnostics.rs#L757

I suggest removing them completely:

-                  new_text: format!("\"{}\"", data.redirect),
+                  new_text: data.redirect.into(),

This should fix the case of @deno-types and triple-slash directives (as their diagnostic.range includes just the URL without its enclosing quotes), but should regress the case of import statements (as their diagnostic.range includes the URL and its enclosing quotes). Therefore I suggest excluding the quotes from specifier_range of import/exports statements:

https://github.com/denoland/deno_graph/blob/ffc6fca09466b07d8e37e560b6546acb6cbd5a48/src/ast.rs#L322

However this might have some far-reaching consequences that I'm not sure about.

[dsherret]

Yeah, the range should not include the quote.

[dsherret]

@relrelb since you diagnosed the problem, would you be interested in contributing the fix?

[nayeemrmn]

Fixed by https://github.com/denoland/deno_graph/pull/220.