feat: add node-builtin-specifier rule

[marvinhagemeister]

Adds a new lint rule that warns when a Node.js built-in module is imported
with a bare specifier (e.g. "fs") instead of the required node: prefix
(e.g. "node:fs"). It covers both static imports and dynamic import()
calls and ships an autofix that adds the node: prefix.

The rule is tagged recommended and emits diagnostics at warning severity
rather than error, so existing code keeps working while nudging users toward
the correct specifier. Warning severity is wired through the
LintDiagnosticSeverity::Warning API added in #1487.

Invalid:

import * as path from "path";
import * as fs from "fs";
import * as fsPromises from "fs/promises";
await import("os");

Valid:

import * as path from "node:path";
import * as fs from "node:fs";
import * as fsPromises from "node:fs/promises";
await import("node:os");

Documentation is added in denoland/docs#3366.

Related denoland/deno#24930

[dsherret]

This looks good. I think though that we should add it to cli/tools/lint/rules in the deno repo instead so that we can keep the node built-in modules in sync with the cli?

[dsherret]

Maybe require-node-prefix or require-node-scheme or something like that?

[marvinhagemeister]

Err... sure, didn't know that it was possible to add lint rules directly to the deno codebase. I'll try porting it.

+1 I like the require-node-prefix naming suggestion

[dsherret]

Another option is making this rule accept a list of built-in node modules that we can provide in the CLI, then we can construct this rule in the CLI's list of rules.

[bartlomieju]

@marvinhagemeister can you move it to the deno repo? Let's land it, it's super useful.

[yacinehmito]

@marvinhagemeister @bartlomieju Is it OK if I move it myself?

[iuioiua]

This looks good. I think though that we should add it to cli/tools/lint/rules in the deno repo instead so that we can keep the node built-in modules in sync with the cli?

Is there anything else to gain from having it in the main repo? I find it funny that lint rules would be split over 2 repos instead of 1.

[bartlomieju]

Thanks! This is a useful rule and there's no overlap with no-import-prefix (which targets inline npm:/jsr:/https: deps). We'd like to take it, defaulting to warning severity rather than error. Could you rebase on latest main and wire it in at warning severity? Once CI is green we can review for merge.

[dsherret]

@bartlomieju did you see the comments on this PR?

[dsherret]

Is there anything else to gain from having it in the main repo? I find it funny that lint rules would be split over 2 repos instead of 1.

Yes, the main thing to gain would be that the node built-in modules can be forced to be kept in sync. It would actually be nice to move this entire repo over there.

For example, this PR is already out of date and is missing the following built-in node modules:

• sqlite
• test
• test/reporters

And these, but questionable to add these or not:

"_http_agent"
"_http_common"
"_http_outgoing"
"_http_server"
"_stream_duplex",
"_stream_passthrough",
"_stream_readable",
"_stream_transform",
"_stream_writable",
"_tls_common",
"_tls_wrap",

[bartlomieju]

Yeah I think node-builtin-specifier name is fine.

[bartlomieju]

Ah I missed the comment about list of modules. That's a good point. I think I will go with the same approach as in #1517. That way it can use the list defined in one of the crates.

It would actually be nice to move this entire repo over there.

Yes, I would like to that in the near future.