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

Bug 500005

Summary: Deleting a quote can cause the whole file to go red...
Product: [ECD] Orion Reporter: Eric Moffatt <emoffatt>
Component: JS ToolsAssignee: Olivier Thomann <Olivier_Thomann>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: curtis.windatt.public, Michael_Rennie, Olivier_Thomann, steve_northover
Version: 12.0   
Target Milestone: 13.0   
Hardware: PC   
OS: Windows 7   
Whiteboard:
Attachments:
Description Flags
This is what I see when I deleted a quote. Olivier thinks it may be due to a new rule he added and will comment
none
Proposed patch none

Description Eric Moffatt CLA 2016-08-19 15:09:13 EDT 
Created attachment 263678 [details]
This is what I see when I deleted a quote. Olivier thinks it may be due to a new rule he added and will comment
Comment 1 Eric Moffatt CLA 2016-08-19 15:12:26 EDT 
Note that the same effect can be induced by deleting  any of: { } ( ) [ ] ' ".
Comment 2 Olivier Thomann CLA 2016-08-19 15:13:26 EDT 
This comes from the new rule "no-used-expressions". It should skip recovered nodes. Will fix it.
Comment 3 Olivier Thomann CLA 2016-08-19 15:58:53 EDT 
Fix is trivial.
Comment 4 Olivier Thomann CLA 2016-08-19 16:09:38 EDT 
Created attachment 263679 [details]
Proposed patch
Comment 5 Olivier Thomann CLA 2016-08-19 16:17:05 EDT 
Added regression test.
Comment 7 Olivier Thomann CLA 2016-08-19 16:31:00 EDT 
Reopen as this might also happen in a file where there is no "recovered" node. This rule should not report problem on a node that has syntax errors. I thought checking for recovered nodes would be enough, but it is not.
Take the file smokeTests/large-broken.js ans remove the closing double quote on line 323. The problem still exists because none of the node in the if statement is tagged as having syntax errors.
Need more work. The previous fix is not wrong. It is just not sufficient.
Comment 8 Olivier Thomann CLA 2016-08-19 16:46:18 EDT 
The only way to skip this rule would to flag the ast node when a syntax error is reported inside the range of a node. This can be quite costly. Needs to be done with cautious.
Comment 9 Olivier Thomann CLA 2016-08-22 09:27:08 EDT 
Ok, I found a way to handle this. If there is a syntax error, the Program node contains an error array that are the syntax errors. So if this array is not empty, I skip the no-unused-expressions check.