Bug 143212 – Web Inspector: Add more ESLint rules that reflect the current state of the code base

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Tobias Reiss]

We should get to a point where ESLint can actually be used as a pre-commit hook, similar to "Tools/Scripts/check-webkit-style". But right now we just have too many linting errors, which is because .eslintrc does not reflect the current state of the code. As soon as we have defined most of the rules, we can fix the errors.

[Radar WebKit Bug Importer]

<rdar://problem/20342932>

[Tobias Reiss]

Created attachment 249712 [details]
patch

[Timothy Hatcher]

Comment on attachment 249712 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=249712&action=review

> Source/WebInspectorUI/.eslintrc:83
> +        "comma-dangle": 0,

I don't like using a comma dangle.

> Source/WebInspectorUI/UserInterface/Models/SourceCodeLocation.js:-214
> -        var currentDisplay = undefined;

This should be legal.

> Source/WebInspectorUI/UserInterface/Views/ObjectTreePropertyTreeElement.js:-360
> -        var prototypeName = undefined;

Ditto.

> Source/WebInspectorUI/UserInterface/Views/TypeTokenView.js:-113
> -        }.bind(this));

This looks like a bug. The function uses this.

> Source/WebInspectorUI/UserInterface/Views/TypeTokenView.js:113
> +        });

Never mind. I see it does not now.

[WebKit Commit Bot]

Comment on attachment 249712 [details]
patch

Clearing flags on attachment: 249712

Committed r182142: <http://trac.webkit.org/changeset/182142>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Tobias Reiss]

Comment on attachment 249712 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=249712&action=review

>> Source/WebInspectorUI/.eslintrc:83
>> +        "comma-dangle": 0,
> 
> I don't like using a comma dangle.

The rule is turned off. If you don't want contributors to use trailing commas than we could set "comma-dangle" to "never".

>> Source/WebInspectorUI/UserInterface/Models/SourceCodeLocation.js:-214
>> -        var currentDisplay = undefined;
> 
> This should be legal.

If you want both to be legal, we could set "no-undef-init" to 0. But keep in mind that `currentDisplay` is undefined by default. There's no need to set it explicitly.

[Joseph Pecoraro]

(In reply to comment #3)
> Comment on attachment 249712 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=249712&action=review
> 
> > Source/WebInspectorUI/.eslintrc:83
> > +        "comma-dangle": 0,
> 
> I don't like using a comma dangle.

I do! It makes diffs easier to read. Also, any good minifier can get rid of the extra commas.

[Tobias Reiss]

Comment on attachment 249712 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=249712&action=review

>>>> Source/WebInspectorUI/.eslintrc:83
>>>> +        "comma-dangle": 0,
>>> 
>>> I don't like using a comma dangle.
>> 
>> The rule is turned off. If you don't want contributors to use trailing commas than we could set "comma-dangle" to "never".
> 
> I do! It makes diffs easier to read. Also, any good minifier can get rid of the extra commas.

ESLint offers 3 variations. "never", "always" and "always-multiline". https://github.com/eslint/eslint/blob/master/docs/rules/comma-dangle.md.