fix: wrong hint for deprecated Deno APIs #1222

skanehira · 2024-01-05T06:03:57Z

The results

$ ./target/debug/examples/dlint run /Users/skanehira/dev/github.com/skanehira/sandbox/deno/stream/main.ts
no-deprecated-deno-api

  × `Deno.readAll` is deprecated and scheduled for removal in Deno 2.0
   ╭─[/Users/skanehira/dev/github.com/skanehira/sandbox/deno/stream/main.ts:2:7]
 2 │ const file = await Deno.open("./main.ts");
 3 │ await Deno.readAll(file);
   ·       ──────┬─────
   ·             ╰── Use `ReadableStream` from https://deno.land/api?s=ReadableStream and `toArrayBuffer()` from https://deno.land/std/streams/to_array_buffer.ts?s=toArrayBuffer instead
 4 │ Deno.iter(file);
   ╰────
  help: https://lint.deno.land/#no-deprecated-deno-api

no-deprecated-deno-api

  × `Deno.iter` is deprecated and scheduled for removal in Deno 2.0
   ╭─[/Users/skanehira/dev/github.com/skanehira/sandbox/deno/stream/main.ts:3:1]
 3 │ await Deno.readAll(file);
 4 │ Deno.iter(file);
   · ────┬────
   ·     ╰── Use `https://deno.land/api?s=ReadableStream` instead
 5 │ Deno.iterSync(file);
   ╰────
  help: https://lint.deno.land/#no-deprecated-deno-api

no-deprecated-deno-api

  × `Deno.iterSync` is deprecated and scheduled for removal in Deno 2.0
   ╭─[/Users/skanehira/dev/github.com/skanehira/sandbox/deno/stream/main.ts:4:1]
 4 │ Deno.iter(file);
 5 │ Deno.iterSync(file);
   · ──────┬──────
   ·       ╰── Use `https://deno.land/api?s=ReadableStream` instead
 6 │ Deno.readAllSync(file);
   ╰────
  help: https://lint.deno.land/#no-deprecated-deno-api

no-deprecated-deno-api

  × `Deno.readAllSync` is deprecated and scheduled for removal in Deno 2.0
   ╭─[/Users/skanehira/dev/github.com/skanehira/sandbox/deno/stream/main.ts:5:1]
 5 │ Deno.iterSync(file);
 6 │ Deno.readAllSync(file);
   · ────────┬───────
   ·         ╰── Use `ReadableStream` from https://deno.land/api?s=ReadableStream and `toArrayBuffer()` from https://deno.land/std/streams/to_array_buffer.ts?s=toArrayBuffer instead
 7 │ Deno.readAll(file);
   ╰────
  help: https://lint.deno.land/#no-deprecated-deno-api

no-deprecated-deno-api

  × `Deno.readAll` is deprecated and scheduled for removal in Deno 2.0
   ╭─[/Users/skanehira/dev/github.com/skanehira/sandbox/deno/stream/main.ts:6:1]
 6 │ Deno.readAllSync(file);
 7 │ Deno.readAll(file);
   · ──────┬─────
   ·       ╰── Use `ReadableStream` from https://deno.land/api?s=ReadableStream and `toArrayBuffer()` from https://deno.land/std/streams/to_array_buffer.ts?s=toArrayBuffer instead
 8 │ Deno.copy(file, file);
   ╰────
  help: https://lint.deno.land/#no-deprecated-deno-api

no-deprecated-deno-api

  × `Deno.copy` is deprecated and scheduled for removal in Deno 2.0
   ╭─[/Users/skanehira/dev/github.com/skanehira/sandbox/deno/stream/main.ts:7:1]
 7 │ Deno.readAll(file);
 8 │ Deno.copy(file, file);
   · ────┬────
   ·     ╰── Use `https://deno.land/api?s=ReadableStream#method_pipeTo_4` instead
 9 │ Deno.write(0, new Uint8Array(1));
   ╰────
  help: https://lint.deno.land/#no-deprecated-deno-api

no-deprecated-deno-api

  × `Deno.writeAll` is deprecated and scheduled for removal in Deno 2.0
    ╭─[/Users/skanehira/dev/github.com/skanehira/sandbox/deno/stream/main.ts:9:1]
  9 │ Deno.write(0, new Uint8Array(1));
 10 │ Deno.writeAll(file, new Uint8Array(1));
    · ──────┬──────
    ·       ╰── Use `WritableStream` from https://deno.land/api?s=WritableStream and `ReadableStream.from` from https://deno.land/api@v1.39.2?s=ReadableStream#variable_ReadableStream and `ReadableStream.pipeTo` from https://
deno.land/api?s=ReadableStream#method_pipeTo_4 instead
    ╰────
  help: https://lint.deno.land/#no-deprecated-deno-api

Found 7 problems

CLAassistant · 2024-01-05T06:04:03Z

All committers have signed the CLA.

magurotuna

Thanks for catching the wrong URLs suggested by the linter. It also seems like the doc is outdated too - would you be interested in working on it? (if not, no worries. I will open an issue in that case)

magurotuna · 2024-01-05T06:17:47Z

src/rules/no_deprecated_deno_api.rs

+      Replacement::NameAndUrls(name_and_urls) => {
+        let mut hint = String::from("Use ");
+        for (i, (name, url)) in name_and_urls.into_iter().enumerate() {
+          if i != 0 {
+            hint.push_str(" and ");
+          }
+          hint.push_str(&format!("`{}` from {}", name, url));
+        }
+        hint.push_str(" instead");
+        hint
+      }


Can we get a unit test for the method hint? It would be great to confirm what hints are generated for complicated cases.

Added unit test 68b0003

Thanks!

I'm a bit worried that putting all the information in one line might look overwhelming to users.

One idea that came up to my mind is to separate hints into multiple lines, where each line contains just one reference to alternative API. Something like this:

Use the following APIs instead - `WritableStream` from https://deno.land/api?s=WritableStream - `ReadableStream.from` from https://deno.land/api?s=ReadableStream#variable_ReadableStream - `ReadableStream.pipeTo` from https://deno.land/api?s=ReadableStream#method_pipeTo_4

But I'm not really sure if this is doable with the current mechanism of showing diagnostics that depends on miette crate.

What do you think?

@magurotuna
Yes, such diagnostic results are indeed easy to understand.
However, I am not familiar with miette, so I don't know if it can output such diagnostic results
Also, since this is a sample implementation, I wonder if I need to work that hard
How about drafting a separate issue from this PR and improving it?
I would like to limit the scope of this issue to fixing the wrong message for now.

That makes sense, we can do further improvement in a follow-up PR. Let's land it as-is then 👍

skanehira · 2024-01-05T08:40:36Z

Thanks for catching the wrong URLs suggested by the linter. It also seems like the doc is outdated too - would you be interested in working on it? (if not, no worries. I will open an issue in that case)

I'll update docs in other PR

magurotuna

LGTM

Fixes #1221 Ref #1222 --------- Co-authored-by: Yusuke Tanaka <yusuktan@maguro.dev>

fix: wrong hint for deprecated Deno APIs)

e716f5f

deno-deploy bot deployed to Preview January 5, 2024 06:04 View deployment

magurotuna reviewed Jan 5, 2024

View reviewed changes

chore: add unittest for deprecated api hint

68b0003

deno-deploy bot had a problem deploying to Preview January 5, 2024 08:39 Failure

skanehira requested a review from magurotuna January 5, 2024 08:41

reset CI

fcf5429

deno-deploy bot deployed to Preview January 7, 2024 23:43 View deployment

chore: fix wrong message

045ca70

deno-deploy bot deployed to Preview January 11, 2024 09:32 View deployment

skanehira mentioned this pull request Jan 11, 2024

docs: update examples in valid section #1226

Merged

magurotuna approved these changes Jan 12, 2024

View reviewed changes

magurotuna merged commit 4dbd182 into denoland:main Jan 12, 2024
6 checks passed

skanehira deleted the fix-wrong-hint branch January 12, 2024 13:55

magurotuna added a commit that referenced this pull request Jan 13, 2024

docs: update examples in valid section (#1226)

9301481

Fixes #1221 Ref #1222 --------- Co-authored-by: Yusuke Tanaka <yusuktan@maguro.dev>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: wrong hint for deprecated Deno APIs #1222

fix: wrong hint for deprecated Deno APIs #1222

skanehira commented Jan 5, 2024 •

edited

Loading

CLAassistant commented Jan 5, 2024 •

edited

Loading

magurotuna left a comment

magurotuna Jan 5, 2024

skanehira Jan 5, 2024

magurotuna Jan 5, 2024

skanehira Jan 11, 2024

magurotuna Jan 12, 2024

skanehira commented Jan 5, 2024

magurotuna left a comment

fix: wrong hint for deprecated Deno APIs #1222

fix: wrong hint for deprecated Deno APIs #1222

Conversation

skanehira commented Jan 5, 2024 • edited Loading

CLAassistant commented Jan 5, 2024 • edited Loading

magurotuna left a comment

Choose a reason for hiding this comment

magurotuna Jan 5, 2024

Choose a reason for hiding this comment

skanehira Jan 5, 2024

Choose a reason for hiding this comment

magurotuna Jan 5, 2024

Choose a reason for hiding this comment

skanehira Jan 11, 2024

Choose a reason for hiding this comment

magurotuna Jan 12, 2024

Choose a reason for hiding this comment

skanehira commented Jan 5, 2024

magurotuna left a comment

Choose a reason for hiding this comment

skanehira commented Jan 5, 2024 •

edited

Loading

CLAassistant commented Jan 5, 2024 •

edited

Loading