deno_lint
deno_lint copied to clipboard
Ignore directives should affect the entire next AST node
As in, the following should be valid:
// deno-lint-ignore no-explicit-any
{
let a: any;
let b: any;
let c: any;
}
This is taken from Rust, #[rustfmt::skip]
and #[allow(warnings)]
work this way. // deno-fmt-ignore
already works this way also. It's much a much nicer way of handling ignore ranges than // deno-lint-ignore-start
etc.
@nayeemrmn very sensible feature, I'm in favor of supporting it. Implementing it will require some refactor to ignore_directive.rs
and filtering diagnostics, but both of them were ripe for a cleanup.
@nayeemrmn I gave this a bit more thought and I'm not so sure anymore - ignoring diagnostic in whole AST means that all diagnostics in children of the node would be ignored as well.
Example:
// deno-lint-ignore no-explicit-any
async function foo(): any {
{
let a: any;
}
...
}
In above example ignore directive above function would also ignore diagnostics inside block. In real world code this might lead to ignoring very deeply placed diagnostics.
Did I get the idea wrong? Could you elaborate a bit more on the use case?
I considered that, I presume the user would have to do this if they really only wanted to capture the return type:
async function foo(): /* deno-lint-ignore no-explicit-any */ any {
{
let a: any; // Fails.
}
...
}
If that's unacceptable, I don't mind making exceptions for functions and similar, or even if this proposal only applied to basic
{}
blocks.
I don't like inline block comments, but I think applying to block nodes makes sense.
In my opinion this should only be done if the comment is above a statement or module item (this is actually how ignore directives work in rslint 👀). Anything else will give you really weird unpredictable scoping. Then after that you can add deno-lint-ignore-next-line
to be more explicit if somebody wants to ignore a whole line. You should reject any directives in that format which are not above a statement
@RDambrosio016 deno-lint-ignore
currently works only on next line. I agree with you on the scoping problem (hence my above comment) but I see value in @nayeemrmn's proposal and I think it does make sense for blocks but only blocks. I really don't want to add more ignore directives, I find it super confusing to have 4 different ignore directives (like eslint does).
I think the intent of the user is important.
I mean, if ecmascript syntax forces the user to place a node on the specific position, like the return type in the code below,
function foo(
a: string
): string {
return a;
}
the ignore directive should go before the ast node, like
// Here
function foo(
a: string
): string {
return a;
}
At the same time, I think directives for parameter should be placed before the line because user can change order of parameters.
function foo(
// Here
a: string
): string {
return a;
}
The same rule can be used for block statements. Block statement is intentional. I mean, the ecmascript syntax does not force the user to create a block statement in a function body.
function foo() {
// deno-lint-ignore no-explicit-any
{
let a: any;
let b: any;
let c: any;
}
}
The block statement in the function is intentionally created, while the block statement of the function is forced syntax. So I think the directive should work like
// Does not affect function body.
function foo() {
// Affect the block statement below.
{
let a: any;
let b: any;
let c: any;
}
}
But sometimes a user may want to ignore the node.
I think another directive like deno-lint-ignore-all
can be used to support the case.
Alternatively, I think using deno-lint-ignore-all
for intentional block statements is also fine.