Opened 11 years ago

Closed 10 years ago

#8010 closed defect (fixed)

Build option stripConsole changes the code logic!

Reported by: lipik Owned by: James Burke
Priority: blocker Milestone: 1.3
Component: BuildTools Version: 1.2.0
Keywords: build, stripConsole Cc:
Blocked By: Blocking:

Description

Hi, I found two cases in my built code today where build had changed the logic. It is because of stripConsole - consider these cases:

var noA = true;
if (a)
  noA = false;
else
  console.log("a is falsey!");
return noA;
function F(v) {
  arguments.length > 0 || console.error("F called with no v");
  G(v);
}

In both these cases, the regexp based removal of console.* (including the trailing semicolon) is a problem.

At the very least, documentation should be updated (as well as runtime help from the build system) to indicate that stripConsole cannot be assumed to have no side-effects apart from removing console calls.

A quick fix would be to not remove the semicolon, in which case the branching would remain correct, though the 2ns case above would break. But that is much better than silent but deadly logic changes.

A proper fix would have to do more than just rely on regular expressions. Also, there are pathological cases like:

console.log(a += b, "hey, are'nt I fresh out of programming school");

Change History (3)

comment:1 Changed 11 years ago by Adam Peller

Component: GeneralBuildTools
Owner: changed from anonymous to James Burke

comment:2 Changed 10 years ago by James Burke

Milestone: tbd1.3

Maybe someday we'll get proper AST generation to help with code pruning. For now, I'll just put a warning in the build help when stripConsole is used.

comment:3 Changed 10 years ago by James Burke

Resolution: fixed
Status: newclosed

(In [16636]) Fixes #8010: put in extra guidance about the stripConsole option.

Note: See TracTickets for help on using tickets.