Exception message for assert(0) depends on whitespace #30872

Activity

1. BridgeAR

   added

   assertIssues and PRs related to the assert subsystem.Issues and PRs related to the assert subsystem.

   confirmed-bugIssues and PRs for confirmed bugs.Issues and PRs for confirmed bugs.

   on Dec 9, 2019
2. BridgeAR commented on Dec 9, 2019

   

   BridgeAR

   on Dec 9, 2019

   Member

   More actions

   @AndrewFinlay thank you for the report. This should indeed not fail to create the better error message.
3. pd4d10 commented on Dec 10, 2019

   

   pd4d10

   on Dec 10, 2019

   Last edited by pd4d10

   Contributor

   More actions

   The left curly bracket without a line break before assert would cause this issue:

   try { assert(0)
   } catch (err) {}
   
   function test() { assert(0)
   }

   It looks like the expression parse is intentionally started at the line start. Modifying it from start to the actual offset seems to fix these cases, but not sure if it breaks others.

   node/lib/assert.js

   Lines 235 to 239 in 7629fb2

   	let start = 0;
   	// Parse the read code until the correct expression is found.
   	do {
   	try {
   	node = parseExpressionAt(code, start, { ecmaVersion: 11 });

   // line 235
   -let start = 0
   +let start = offset

   😕React with 😕1Reacted by Alex Yang
4. himself65 commented on Dec 10, 2019

   

   himself65

   on Dec 10, 2019

   Member

   More actions

   @pd4d10 no

   node/lib/assert.js

   Lines 239 to 242 in 7629fb2

   	node = parseExpressionAt(code, start, { ecmaVersion: 11 });
   	start = node.end + 1 || start;
   	// Find the CallExpression in the tree.
   	node = findNodeAround(node, offset, 'CallExpression');

    node = parseExpressionAt(code, start, { ecmaVersion: 11 });

   this line will throw error with message Unexpected token (1:2) and the code are

   try {         assert(condition, message)
     } catch (e) {
       throw e
     }
   }
   
   // Now run tests
   
   console.log('Call assert proxy with slightly minified whitespace')
   try {
     assertProxySlightlyMinified(0)
     console.error(`assertProxySlightlyMinified(0); failed to cause an exception`)
   } catch (e) {
     console.log(`assertProxySlightlyMinified(0); generated exception: ${e}`)
   }
   
   console.log('Finished tests')
   
5. himself65 commented on Dec 10, 2019

   

   himself65

   on Dec 10, 2019

   Member

   More actions

   and this code will not throw error

       assert(condition, message)
     } catch (e) {
       throw e
     }
   }
   
   const assertProxySlightlyMinified = (condition, message) => {
     try {         assert(condition, message)
     } catch (e) {
       throw e
     }
   }
   
   // Now run tests
   
   console.log('Call assert proxy with slightly minified whitespace')
   try {
     assertProxy(0)
     console.error(`assertProxySlightlyMinified(0); failed to cause an exception`)
   } catch (e) {
     console.log(`assertProxySlightlyMinified(0); generated exception: ${e}`)
   }
   
   console.log('Finished tests')
   
6. BridgeAR commented on Dec 10, 2019

   

   BridgeAR

   on Dec 10, 2019

   Last edited by BridgeAR

   Member

   More actions

   @himself65 @pd4d10 is correct about that. The start is set to zero to include assert or what ever name the user named it. It would otherwise only show ok().

   I am currently looking into it (there is an easy solution but that would waste a lot of CPU time, so I am trying to find a proper fix).

   👍React with 👍1Reacted by Alex Yang
7. BridgeAR

   self-assigned this

   on Dec 10, 2019