feat(linter-rules/no-unused-vars): declared var must be used #2955
Conversation
sidvishnoi
force-pushed
the
linter/no-unused-vars
branch
from
f1a4d0f to
45b51cb
Compare
|
|
||
| const name = "no-unused-vars"; | ||
|
|
||
| const meta = { |
There was a problem hiding this comment.
Make sure the message here looks like one you would get from ESLint... that is, it should be clear that a "variable X was defined but never used".
There was a problem hiding this comment.
We won't have access to "X" in linter message, so maybe:
"Variable was defined, but never used. Add a data-ignore-unused attribute to the <var>, or consider using <em>/ <code>"
There was a problem hiding this comment.
"Add a data-ignore-unused attribute to the , or consider using /
"
Only the "data-ignore-unused" seems like an good suggestion... the other not so much as they are working around the problem, not addressing the problem explicitly.
There was a problem hiding this comment.
No idea what's going on with my comment's formatting above... 😂
There was a problem hiding this comment.
Only the "data-ignore-unused" seems like an good suggestion... the other not so much as they are working around the problem, not addressing the problem explicitly.
Maybe author used <var> for its style when they meant <em> or <code> (See https://github.com/w3c/webdriver/pull/1515/files)?
No idea what's going on with my comment's formatting above... 😂
Missed backticks on <em> 🎈
There was a problem hiding this comment.
Also, we can't add a data-ignore-unused to the var micro-syntax.
There was a problem hiding this comment.
Using <var> for style is an author error, so let’s not worry about it (or encourage that).
| function linterFunction(_, doc) { | ||
| const offendingElements = []; | ||
|
|
||
| for (const section of doc.querySelectorAll("section")) { |
There was a problem hiding this comment.
Should we pick only those sections that contain algorithms?
See: https://github.com/w3c/payment-request/blob/e9515622/index.html#L280
There was a problem hiding this comment.
I think so... we might need to test out a few specs.
| * ``` | ||
| * The selector does not match: | ||
| * ``` html | ||
| * <section><div><ul class="algorithm"></ul></div></section> |
There was a problem hiding this comment.
Should this be supported? Seems unlikely, but possible
There was a problem hiding this comment.
Heh, commented at the same time 😂 yes, I think we should support it
| * ``` | ||
| * The selector does not match: | ||
| * ``` html | ||
| * <section><div><ul class="algorithm"></ul></div></section> |
There was a problem hiding this comment.
What’s the rationale for this one?
There was a problem hiding this comment.
The selector gets complex (match section only if section contains algorithm, but not any subsection). Looking at specs, most .algorithm were in directly section, not wrapped in other elements. Might go the imperative way if selector gets too complex and we want to support it.
Based on feedback from speced/bikeshed#655 (comment)
Wiki: https://github.com/w3c/respec/wiki/no-unused-vars