Fixing eslint errors when migrating test .xul files to .xhtml

[Brian Grinstead]

I noticed when migrating test files from .xul->.xhtml in Bug 1579952 that the file rename causes us to start eslinting the tests. I presume this means that we aren’t linting the contents of <script> tags in XUL docs but we are in XHML. This leads to a bunch of failures (for instance, here are some failures when migrating the accessible/ folder: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a678bc958e875e4117e4118aa5d1869a8097d0d&selectedJob=267352075).

Most of these aren't currently fixable with `./mach eslint —fix` - we go from 32 -> 30 errors on that patch after running fix. I'm wondering if there's anything we can do here to avoid injecting manual JS changes into the process of renaming files. For instance:
- could we allow --fix to work on more rules?
- are there other tools that we could use to help autofix these types of errors?
- should we make eslint ignore these failing files?
- are we doing something special with .xul files in tree that we should also do with .xhtml?

Thanks,
Brian

_______________________________________________
firefox-dev mailing list
firef...@mozilla.org
https://mail.mozilla.org/listinfo/firefox-dev

[Dave Townsend]

So apparently the XUL pre-processer we use for eslint doesn't support inline scripts, the errors you're seeing are from code that just wasn't being linted before. I think it would be fine for now to just add them to the ignored list but it would be good to put a bug on file to get them fixed up though.

As for making autofix work better, most of the rules you're hitting are in the eslint source so would need to be fixed upstream and from what I can see a lot are of the form where it is difficult to know what the correct change would be automatically.

[Mark Banner]

On 19/09/2019 22:20, Dave Townsend wrote:

So apparently the XUL pre-processer we use for eslint doesn't support inline scripts, the errors you're seeing are from code that just wasn't being linted before. I think it would be fine for now to just add them to the ignored list but it would be good to put a bug on file to get them fixed up though.

The XUL preprocessor also disables some rules iirc. The preprocessing was added to get at least some linting there, but with xul going away we didn't want to do lots of work on it.

The best way to "fix" these is probably adding an entry to the top-level .eslintrc.js file, and disable just the rules that are failing for that directory. That way, we get the majority of rules enabled, and can work on the remaining ones (these tend to make good mentored bugs).

You'll see that there's already various directory/rule combinations that we have disabled currently, and there's various bugs in place to get these enabled.

Mark