BenoitZugmeyer / eslint-plugin-html

An ESLint plugin to extract and lint scripts from HTML files.
ISC License
430 stars 51 forks source link

inline event handler linting #123

Open brettz9 opened 4 years ago

brettz9 commented 4 years ago

Description

Inline event handlers such as onload can contain JavaScript. These may contain errors or one may not want these handlers at all.

It would be great if eslint-plugin-html:

  1. Could expose their content to linting
  2. Could provide its own rule to optionally flag against any use of inline event handlers
  3. Could tweak rules such that it is recognized these handlers (effectively?) run within a function:*
    1. For all inline event handlers treat event as defined (and for onerror treat source, lineno, colno, and error as defined) (but don't treat them as unused if they are not used).
    2. Allow this
    3. Allow a return value

Alternatives

Allow bad code within HTML. (A particular concern is the likes of https://github.com/mozilla/eslint-plugin-no-unsanitized not being able to find innerHTML usage.)

There was at one point a Firefox add-on for detecting inline handlers, but this is not automated.

Additional context

* See -https://developer.mozilla.org/en-US/docs/Web/Guide/Events/Event_handlers#Event_handlers_parameters_this_binding_and_the_return_value

And thanks for the great plugin!

BenoitZugmeyer commented 4 years ago

First of all, thank you for the well written issue and sorry for the response delay.

This is a popular issue (see previous one https://github.com/BenoitZugmeyer/eslint-plugin-html/issues/61), even an implementation attempt have been made (https://github.com/BenoitZugmeyer/eslint-plugin-html/pull/87) but never followed. That said, I'm a bit shy about working on this, as there is a few open questions:

For all inline event handlers treat event as defined

As far as the browser environment is enabled, this shouldn't be a problem since event is already a global variable

brettz9 commented 4 years ago

First of all, thank you for the well written issue and sorry for the response delay.

All good, thank you for the reply!

This is a popular issue (see previous one #61), even an implementation attempt have been made (#87) but never followed. That said, I'm a bit shy about working on this, as there is a few open questions:

* how would rules like `indent` work for this?

I'd think either disabled, or if there may be some use cases for some questionable rules, leave it to overrides to disable. I don't know if you are using the APIs required for this, but at https://eslint.org/docs/user-guide/configuring#specifying-processor , it mentions:

Processors may make named code blocks such as 0.js and 1.js. ESLint handles such a named code block as a child file of the original file. You can specify additional configurations for named code blocks in the overrides section of the config.

* how would scope work? Ex: if we define a variable in an event handler, should it be considered as defined in other script tags? Module script tags?
<body>

<!-- The following won't work with type=module, whether the input is above or below the script,
but the input does work in either position when there is no type=module -->
<input onchange="alert(a);">
<script type="module">
var a = 5;
</script>

<!-- This implicit global can affect the script below, whether it is type=module or not, and
whether the input is below or above; it will be an error until defined -->
<input onchange="b = 8;">
<script type="module">
setInterval(() => {
  console.log(b);
}, 3000);
</script>

</body>

Perhaps markVariableAsUsed can be used--the apparently only ESLint feature that allows shared state between rules (not sure how it works with processors).

For all inline event handlers treat event as defined

As far as the browser environment is enabled, this shouldn't be a problem since event is already a global variable

Cool