Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 499665

Summary: [eslint] update "semi" rule.
Product: [ECD] Orion Reporter: Olivier Thomann <Olivier_Thomann>
Component: JS ToolsAssignee: Olivier Thomann <Olivier_Thomann>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: curtis.windatt.public, Michael_Rennie
Version: 12.0   
Target Milestone: 13.0   
Hardware: PC   
OS: Windows 7   
Whiteboard:
Attachments:
Description Flags
Current patch
none
Proposed patch none

Description Olivier Thomann CLA 2016-08-15 10:25:20 EDT 
This rule has changed lately to add a "never" option in order to make sure there is never a semi-colon. We don't support that "never" option. So if the user passes in a .eslintrc file with semi set to [2, "never"], we report an error for each missing semi-colon when we should ignore them since this is exactly what the user expects.
I'll update that before moving to eslint 3.0.1.
Comment 1 Olivier Thomann CLA 2016-08-15 15:22:38 EDT 
So with the new implementation of this rule, "semi" can now return extra semicolon or missing semicolon.
Our quickfix only checks the annotation id to be triggered. So I need to "emulate" a new rule id (extra-semi) when reporting errors for extra semicolons even if I report a "semi" problem.
For example,
/*eslint semi: ["error", "always", { "omitLastInOneLineBlock": true}] */
if (foo) { bar(); }

This is now a problem for having an extra semicolon. I'll try to reuse the "no-extra-semi" rule to report this one.
Comment 2 Olivier Thomann CLA 2016-08-15 15:46:58 EDT 
I can reuse the existing "no-extra-semi" rule and quickfix. My only issue is that if this has been disabled by the user in the settings page, it can still appear as the severity is based on the severity set for the "semi" rule.
Our existing quickfix for "semi" is only expecting to "add" missing semi-colon. It doesn't expect to remove some. Also in the validator code, we "get" again the error message based on the key we pass in (rule id). I don't like that. We should rely on the rule implementation to report the appropriate message. It looks like this has been done to support substitution into the error message like to the eqeqeq rule.
Comment 3 Olivier Thomann CLA 2016-08-15 16:14:23 EDT 
Created attachment 263603 [details]
Current patch
Comment 4 Olivier Thomann CLA 2016-08-16 11:48:14 EDT 
Created attachment 263617 [details]
Proposed patch