Make more JS files beter match the coding standard. (issue 8888006)
l...@chromium.org
Message:
Have fun :)
Description:
Make more JS files beter match the coding standard.
This includes adding missing semicolons and braces, removing superflous
semicolons and breaking long lines.
Please review this at http://codereview.chromium.org/8888006/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/array.js
M src/d8.js
M src/v8natives.js
M test/mjsunit/apply.js
M test/mjsunit/argument-named-arguments.js
M test/mjsunit/arguments-apply.js
M test/mjsunit/arguments-enum.js
M test/mjsunit/arguments-escape.js
M test/mjsunit/arguments-read-and-assignment.js
M test/mjsunit/arguments.js
M test/mjsunit/array-concat.js
M test/mjsunit/array-constructor.js
M test/mjsunit/array-elements-from-array-prototype-chain.js
M test/mjsunit/array-elements-from-array-prototype.js
M test/mjsunit/array-elements-from-object-prototype.js
M test/mjsunit/array-functions-prototype-misc.js
M test/mjsunit/array-functions-prototype.js
M test/mjsunit/array-indexing.js
M test/mjsunit/array-iteration.js
M test/mjsunit/array-reduce.js
M test/mjsunit/array-slice.js
M test/mjsunit/array-sort.js
M test/mjsunit/array-splice.js
M test/mjsunit/array-unshift.js
M test/mjsunit/assert-opt-and-deopt.js
M test/mjsunit/break.js
M test/mjsunit/bugs/618.js
M test/mjsunit/bugs/bug-222.js
M test/mjsunit/call-non-function.js
M test/mjsunit/class-of-builtins.js
M test/mjsunit/closure.js
M test/mjsunit/closures.js
M test/mjsunit/compiler/assignment-deopt.js
M test/mjsunit/compiler/call-keyed.js
M test/mjsunit/compiler/compare.js
M test/mjsunit/compiler/count-deopt.js
M test/mjsunit/compiler/global-accessors.js
M test/mjsunit/compiler/inline-context-slots.js
M test/mjsunit/compiler/inline-two.js
M test/mjsunit/compiler/literals.js
M test/mjsunit/compiler/null-compare.js
M test/mjsunit/compiler/objectliterals.js
M test/mjsunit/compiler/pic.js
M test/mjsunit/compiler/regress-1394.js
M test/mjsunit/compiler/regress-2.js
M test/mjsunit/compiler/regress-3249650.js
M test/mjsunit/compiler/regress-5.js
M test/mjsunit/compiler/regress-8.js
M test/mjsunit/compiler/regress-closures-with-eval.js
M test/mjsunit/compiler/regress-inline-callfunctionstub.js
M test/mjsunit/compiler/regress-lbranch-double.js
M test/mjsunit/compiler/regress-serialized-slots.js
M test/mjsunit/compiler/regress-stacktrace-methods.js
M test/mjsunit/compiler/short-circuit.js
M test/mjsunit/compiler/simple-bailouts.js
M test/mjsunit/compiler/simple-binary-op.js
M test/mjsunit/compiler/simple-global-access.js
M test/mjsunit/compiler/simple-inlining.js
M test/mjsunit/compiler/thisfunction.js
M test/mjsunit/const-redecl.js
M test/mjsunit/constant-folding.js
M test/mjsunit/context-variable-assignments.js
M test/mjsunit/d8-os.js
M test/mjsunit/date-parse.js
M test/mjsunit/date.js
M test/mjsunit/debug-backtrace-text.js
M test/mjsunit/debug-backtrace.js
M test/mjsunit/debug-breakpoints.js
M test/mjsunit/debug-changebreakpoint.js
M test/mjsunit/debug-clearbreakpoint.js
M test/mjsunit/debug-clearbreakpointgroup.js
M test/mjsunit/debug-compile-event-newfunction.js
M test/mjsunit/debug-compile-event.js
M test/mjsunit/debug-conditional-breakpoints.js
M test/mjsunit/debug-constructed-by.js
M test/mjsunit/debug-constructor.js
M test/mjsunit/debug-continue.js
M test/mjsunit/debug-enable-disable-breakpoints.js
M test/mjsunit/debug-evaluate-arguments.js
M test/mjsunit/debug-evaluate-bool-constructor.js
M test/mjsunit/debug-evaluate-locals-optimized-double.js
M test/mjsunit/debug-evaluate-locals-optimized.js
M test/mjsunit/debug-evaluate-locals.js
M test/mjsunit/debug-evaluate-recursive.js
M test/mjsunit/debug-evaluate-with-context.js
M test/mjsunit/debug-evaluate-with.js
M test/mjsunit/debug-evaluate.js
M test/mjsunit/debug-event-listener.js
M test/mjsunit/debug-handle.js
M test/mjsunit/debug-ignore-breakpoints.js
M test/mjsunit/debug-listbreakpoints.js
M test/mjsunit/debug-liveedit-1.js
M test/mjsunit/debug-liveedit-2.js
M test/mjsunit/debug-liveedit-3.js
M test/mjsunit/debug-liveedit-breakpoints.js
M test/mjsunit/debug-liveedit-check-stack.js
M test/mjsunit/debug-liveedit-diff.js
M test/mjsunit/debug-liveedit-newsource.js
M test/mjsunit/debug-liveedit-patch-positions-replace.js
M test/mjsunit/debug-liveedit-patch-positions.js
M test/mjsunit/debug-liveedit-utils.js
test/mjsunit/debug-mirror-cache.js
M test/mjsunit/debug-multiple-breakpoints.js
M test/mjsunit/debug-receiver.js
M test/mjsunit/debug-referenced-by.js
M test/mjsunit/debug-references.js
M test/mjsunit/debug-return-value.js
M test/mjsunit/debug-scopes.js
M test/mjsunit/debug-script-breakpoints.js
M test/mjsunit/debug-script.js
M test/mjsunit/debug-scripts-request.js
test/mjsunit/debug-setbreakpoint.js
M test/mjsunit/debug-setexceptionbreak.js
test/mjsunit/debug-sourceinfo.js
M test/mjsunit/debug-sourceslice.js
M test/mjsunit/debug-step-2.js
M test/mjsunit/debug-step-3.js
M test/mjsunit/debug-step-stub-callfunction.js
M test/mjsunit/debug-step.js
M test/mjsunit/debug-stepin-accessor.js
M test/mjsunit/debug-stepin-builtin.js
M test/mjsunit/debug-stepin-call-function-stub.js
M test/mjsunit/debug-stepin-constructor.js
M test/mjsunit/debug-stepin-function-call.js
M test/mjsunit/debug-stepnext-do-while.js
M test/mjsunit/debug-stepout-recursive-function.js
M test/mjsunit/debug-stepout-to-builtin.js
M test/mjsunit/debug-suspend.js
M test/mjsunit/debug-version.js
M test/mjsunit/div-mod.js
M test/mjsunit/elements-kind.js
test/mjsunit/error-constructors.js
M test/mjsunit/escape.js
M test/mjsunit/eval-enclosing-function-name.js
M test/mjsunit/eval.js
M test/mjsunit/extra-arguments.js
M test/mjsunit/extra-commas.js
M test/mjsunit/for-in-delete.js
M test/mjsunit/for-in.js
M test/mjsunit/function-bind.js
M test/mjsunit/function-call.js
M test/mjsunit/function-property.js
M test/mjsunit/function-prototype.js
M test/mjsunit/function-source.js
M test/mjsunit/function.js
M test/mjsunit/fuzz-natives.js
M test/mjsunit/get-own-property-descriptor.js
M test/mjsunit/get-prototype-of.js
M test/mjsunit/html-comments.js
M test/mjsunit/html-string-funcs.js
M test/mjsunit/if-in-undefined.js
M test/mjsunit/indexed-accessors.js
M test/mjsunit/indexed-value-properties.js
M test/mjsunit/instanceof-2.js
M test/mjsunit/json.js
M test/mjsunit/keyed-call-generic.js
M test/mjsunit/keyed-call-ic.js
M test/mjsunit/keyed-ic.js
M test/mjsunit/leakcheck.js
M test/mjsunit/limit-locals.js
M test/mjsunit/math-abs.js
M test/mjsunit/megamorphic-callbacks.js
M test/mjsunit/mirror-array.js
M test/mjsunit/mirror-error.js
M test/mjsunit/mirror-function.js
M test/mjsunit/mirror-object.js
M test/mjsunit/mirror-regexp.js
M test/mjsunit/mirror-string.js
test/mjsunit/mirror-unresolved-function.js
M test/mjsunit/no-semicolon.js
M test/mjsunit/nul-characters.js
M test/mjsunit/number-string-index-call.js
M test/mjsunit/object-create.js
M test/mjsunit/object-define-property.js
M test/mjsunit/object-freeze.js
M test/mjsunit/object-get-own-property-names.js
M test/mjsunit/object-literal-overwrite.js
M test/mjsunit/object-literal.js
M test/mjsunit/object-prevent-extensions.js
M test/mjsunit/object-seal.js
M test/mjsunit/object-toprimitive.js
M test/mjsunit/optimized-typeof.js
M test/mjsunit/override-read-only-property.js
M test/mjsunit/property-load-across-eval.js
M test/mjsunit/property-object-key.js
M test/mjsunit/prototype.js
M test/mjsunit/receiver-in-with-calls.js
A test/mjsunit/regexp-capture.js
M test/mjsunit/regexp-captures.js
M test/mjsunit/regexp-lookahead.js
M test/mjsunit/regexp-static.js
M test/mjsunit/regexp.js
M test/mjsunit/regress/regress-1015.js
M test/mjsunit/regress/regress-1030466.js
M test/mjsunit/regress/regress-1036894.js
M test/mjsunit/regress/regress-1062422.js
M test/mjsunit/regress/regress-1081309.js
M test/mjsunit/regress/regress-1099.js
M test/mjsunit/regress/regress-1106.js
M test/mjsunit/regress/regress-1118.js
[[ 105 additional files ]]
ri...@chromium.org
I see 4 or 5 files that are empty here (both before and after view), e.g.,
mjsunit/substr.js. These are not empty in my checkout, please make sure to
not
delete those.
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js
File test/mjsunit/arguments-read-and-assignment.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js#newcode30
test/mjsunit/arguments-read-and-assignment.js:30: assertEquals(42,
function(){ return arguments;}(42)[0],
space after arguments;
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js#newcode32
test/mjsunit/arguments-read-and-assignment.js:32: assertEquals(42,
function(){ return arguments;}(42)[0],
space after arguments;
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js#newcode36
test/mjsunit/arguments-read-and-assignment.js:36: assertEquals(42,
function(){ if(arguments) return 42;}(),
space after 42;
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js#newcode38
test/mjsunit/arguments-read-and-assignment.js:38: assertEquals(42,
function(){ return arguments || true;}(42)[0],
space after true;
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js#newcode40
test/mjsunit/arguments-read-and-assignment.js:40: assertEquals(true,
function(){ return arguments && [true];}(42)[0],
space after [true];
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js#newcode42
test/mjsunit/arguments-read-and-assignment.js:42: assertEquals(42,
function(){ arguments = 42; return 42;}(),
space after 42;
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js#newcode74
test/mjsunit/arguments-read-and-assignment.js:74:
In all of the above I would prefer having the function on its own line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js#newcode125
test/mjsunit/arguments-read-and-assignment.js:125: arguments[1] = 11;
new line before function, and move last argument to assertEquals down on
individual line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js#newcode154
test/mjsunit/arguments-read-and-assignment.js:154: }() + b;
indention
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js#newcode159
test/mjsunit/arguments-read-and-assignment.js:159: function
weirdargs(a,b,c) { if (!a) return arguments;
new line before function body
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js#newcode160
test/mjsunit/arguments-read-and-assignment.js:160: return [b[2],c]; }
move } down
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-concat.js
File test/mjsunit/array-concat.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-concat.js#newcode207
test/mjsunit/array-concat.js:207: arr2.push("X");
indention (I don't think 4 indent + 2 for the function makes a lot of
sense here :-) )
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-concat.js#newcode216
test/mjsunit/array-concat.js:216: arr2[500000] = "X";
indention
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-concat.js#newcode228
test/mjsunit/array-concat.js:228: function mkGetter(i) { return
function() { trace.push(i); }; }
remove last semicolon
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-elements-from-array-prototype-chain.js
File test/mjsunit/array-elements-from-array-prototype-chain.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-elements-from-array-prototype-chain.js#newcode72
test/mjsunit/array-elements-from-array-prototype-chain.js:72: //
Side-effects: Array.prototype[3] now percolates into a[5] and
Array.prototype[7]
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-iteration.js
File test/mjsunit/array-iteration.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-iteration.js#newcode53
test/mjsunit/array-iteration.js:53: assertArrayEquals([42,42],
a.filter(function(n, index, array) { array[index] = 43; return 42 == n;
}));
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-iteration.js#newcode59
test/mjsunit/array-iteration.js:59: assertArrayEquals([],
a.filter(function(n, index, array) { array.push(n+1); return n == 2;
}));
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-iteration.js#newcode182
test/mjsunit/array-iteration.js:182: assertArrayEquals(result,
a.map(function(n, index, array) { array.push(n); return n + 1;}));
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-iteration.js#newcode212
test/mjsunit/array-iteration.js:212: assertTrue(a.some(function(n,
index, array) { array[index] = n + 1; return n == 2; }));
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-iteration.js#newcode217
test/mjsunit/array-iteration.js:217: assertFalse(a.some(function(n,
index, array) { array.push(42); return n == 42; }));
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/compiler/null-compare.js
File test/mjsunit/compiler/null-compare.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/compiler/null-compare.js#newcode29
test/mjsunit/compiler/null-compare.js:29: if (x == null) { return true;
this did not get much better :-)
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/compiler/null-compare.js#newcode39
test/mjsunit/compiler/null-compare.js:39: if (x === null) { return true;
neither did this
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/compiler/regress-2.js
File test/mjsunit/compiler/regress-2.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/compiler/regress-2.js#newcode31
test/mjsunit/compiler/regress-2.js:31: {
move this up as well
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/compiler/regress-5.js
File test/mjsunit/compiler/regress-5.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/compiler/regress-5.js#newcode35
test/mjsunit/compiler/regress-5.js:35: if (y == 0) { break bar;
put on new line and use {} in else as well
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/compiler/regress-8.js
File test/mjsunit/compiler/regress-8.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/compiler/regress-8.js#newcode27
test/mjsunit/compiler/regress-8.js:27:
This file needs more love:
space after ,
space around +
space around ==
space around =
space round >=
jspace after if
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/compiler/regress-8.js#newcode44
test/mjsunit/compiler/regress-8.js:44: function O() { this.append =
function(a,b,c,d,e) { return a + b + c + d + e; }; }
long line (and the looks better on individual line no matter what)
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/compiler/regress-8.js#newcode85
test/mjsunit/compiler/regress-8.js:85:
LA+(a.Un+(zE+(Fp+(LA+(a.Im+(zE+(Mob+(LA+(a.total+zE))))))))),
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/compiler/regress-serialized-slots.js
File test/mjsunit/compiler/regress-serialized-slots.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/compiler/regress-serialized-slots.js#newcode38
test/mjsunit/compiler/regress-serialized-slots.js:38: {
move { up (or function down), also indention below seems wrong with this
here, I would move the function down with 4 indent and then the normal 2
indent for function body, alternatively, just move } up and then have 2
indent
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace-text.js
File test/mjsunit/debug-backtrace-text.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace-text.js#newcode76
test/mjsunit/debug-backtrace-text.js:76: if (event ==
Debug.DebugEvent.Break) {
indention
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace-text.js#newcode83
test/mjsunit/debug-backtrace-text.js:83:
assertEquals("#<Point>.distanceTo(p=#<Point>)",
exec_state.frame(0).invocationText());
long line + two next lines
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace-text.js#newcode94
test/mjsunit/debug-backtrace-text.js:94: assertEquals("createPoint(x=0,
y=0)", exec_state.frame(1).invocationText());
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js
File test/mjsunit/debug-backtrace.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode126
test/mjsunit/debug-backtrace.js:126: json =
'{"seq":0,"type":"request","command":"backtrace","arguments":{"fromFrame":1,"toFrame":3}}';
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode144
test/mjsunit/debug-backtrace.js:144: json =
'{"seq":0,"type":"request","command":"backtrace","arguments":{"fromFrame":0,"toFrame":2,
"bottom":true}}';
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode176
test/mjsunit/debug-backtrace.js:176: json =
'{"seq":0,"type":"request","command":"frame","arguments":{"number":0}}';
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode185
test/mjsunit/debug-backtrace.js:185: assertEquals('number',
response.lookup(frame.arguments[0].value.ref).type);
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode190
test/mjsunit/debug-backtrace.js:190: json =
'{"seq":0,"type":"request","command":"frame","arguments":{"number":1}}';
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode201
test/mjsunit/debug-backtrace.js:201: json =
'{"seq":0,"type":"request","command":"frame","arguments":{"number":3}}';
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-breakpoints.js
File test/mjsunit/debug-breakpoints.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-breakpoints.js#newcode32
test/mjsunit/debug-breakpoints.js:32: function f() {a=1;b=2;}
move body to (two) individual lines and space around =
if this is due to breakpoint positions just leave as is
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-breakpoints.js#newcode37
test/mjsunit/debug-breakpoints.js:37:
there seems to be missing a lot of spaces below, if this is to ease the
tests depending on position I am fine with leaving this as is
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-changebreakpoint.js
File test/mjsunit/debug-changebreakpoint.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-changebreakpoint.js#newcode72
test/mjsunit/debug-changebreakpoint.js:72: testArguments(dcp,
'{"breakpoint":' + (breakpoint + 1) + ',"condition":"false"}', false);
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-version.js
File test/mjsunit/debug-version.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-version.js#newcode64
test/mjsunit/debug-version.js:64:
assertTrue(!!(version_string.match(version_pattern)), "unexpected format
of version: " + version_string);
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/div-mod.js
File test/mjsunit/div-mod.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/div-mod.js#newcode46
test/mjsunit/div-mod.js:46: var div_func = this.eval("(function(left) {
return left / " + divisor + "; })");
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/div-mod.js#newcode47
test/mjsunit/div-mod.js:47: var mod_func = this.eval("(function(left) {
return left % " + divisor + "; })");
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/div-mod.js#newcode100
test/mjsunit/div-mod.js:100: if (dividend < 0) { dividend = -dividend;
sign = -1; }
body on individual lines
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/div-mod.js#newcode207
test/mjsunit/div-mod.js:207: mod_func = this.eval("(function(left) {
return left % " + divisors[j]+ "; })");
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/div-mod.js#newcode208
test/mjsunit/div-mod.js:208: assertEquals((mod_func)(left_operands[i]),
left_operands[i] % divisors[j]);
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/div-mod.js#newcode209
test/mjsunit/div-mod.js:209: assertEquals((mod_func)(-left_operands[i]),
-left_operands[i] % divisors[j]);
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/div-mod.js#newcode215
test/mjsunit/div-mod.js:215: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/div-mod.js#newcode217
test/mjsunit/div-mod.js:217: [0, 0, 0, 8, 24, 56, 120, 120, 120, 632,
1656, 1656, 5752, 5752, 22136, 22136, 22136, 22136, 284280, 284280,
1332856, 3430008, 3430008, 3430008, 3430008, 36984440, 36984440,
36984440, 305419896, 305419896, 305419896],
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/for-in.js
File test/mjsunit/for-in.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/for-in.js#newcode61
test/mjsunit/for-in.js:61: a[Math.pow(2,30)-1] = 0;
space after , and around -
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/for-in.js#newcode63
test/mjsunit/for-in.js:63: a[Math.pow(2,31)-1] = 0;
space after , and around -
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/for-in.js#newcode90
test/mjsunit/for-in.js:90: a = [1,2,3,4];
space after ,
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/html-comments.js
File test/mjsunit/html-comments.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/html-comments.js#newcode47
test/mjsunit/html-comments.js:47: var x = 1; x <! x--;
why not ; here and below
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/json.js
File test/mjsunit/json.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/json.js#newcode30
test/mjsunit/json.js:30: assertEquals("1979-01-11T08:00:00.000Z", new
Date("1979-01-11 08:00 GMT").toJSON());
long line + below
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/json.js#newcode339
test/mjsunit/json.js:339: }
above: move body down to individual lines + move else if up on same line
as }
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/json.js#newcode340
test/mjsunit/json.js:340: else if (string == '\r') expected = '\\r';
also use {} for last else if and move up on previous line after {
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/leakcheck.js
File test/mjsunit/leakcheck.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/leakcheck.js#newcode34
test/mjsunit/leakcheck.js:34: if (n > 0) { return fac(n - 1) * n;
body down, else up and {} in else
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/no-semicolon.js
File test/mjsunit/no-semicolon.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/no-semicolon.js#newcode30
test/mjsunit/no-semicolon.js:30:
I don't think we wan't to add these in this file, see comment above
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/number-string-index-call.js
File test/mjsunit/number-string-index-call.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/number-string-index-call.js#newcode28
test/mjsunit/number-string-index-call.js:28: var callbacks = [
function() {return 'foo';}, "nonobject", /abc/ ];
space after { and before }
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/object-create.js
File test/mjsunit/object-create.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/object-create.js#newcode66
test/mjsunit/object-create.js:66: function valueGet() { ctr5++; return
3; }
individual lines for body
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/object-create.js#newcode67
test/mjsunit/object-create.js:67: function getterGet() { ctr5++; return
function() { return ctr6++; }; }
individual lines for body
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/object-define-property.js
File test/mjsunit/object-define-property.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/object-define-property.js#newcode395
test/mjsunit/object-define-property.js:395: function get(){return
this.x;}
spaces around the body + below (I wonder who wrote this :-) )
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-1099.js
File test/mjsunit/regress/regress-1099.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-1099.js#newcode30
test/mjsunit/regress/regress-1099.js:30: // Test that LApplyArguments
lithium instruction restores context after the call.
long line
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-1170187.js
File test/mjsunit/regress/regress-1170187.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-1170187.js#newcode76
test/mjsunit/regress/regress-1170187.js:76: (function(x,y){var a,b,c;
debugger; return 3;})();
spaces after ,
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-1207276.js
File test/mjsunit/regress/regress-1207276.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-1207276.js#newcode33
test/mjsunit/regress/regress-1207276.js:33: function
X(){String(Glo0al);}
space around body
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-1257.js
File test/mjsunit/regress/regress-1257.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-1257.js#newcode37
test/mjsunit/regress/regress-1257.js:37: case -1:
indention
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-294.js
File test/mjsunit/regress/regress-294.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-294.js#newcode35
test/mjsunit/regress/regress-294.js:35: if (x == "kat") { x = "kat";
body down else up
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-475.js
File test/mjsunit/regress/regress-475.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-475.js#newcode28
test/mjsunit/regress/regress-475.js:28: assertEquals(1, (function
(){return 1|-1%1;})());
spaces around body of function and around operators
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-524.js
File test/mjsunit/regress/regress-524.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-524.js#newcode32
test/mjsunit/regress/regress-524.js:32: for (var j = 0; j < i; j++) {
var o = {}; o.x = 42; delete o.x; a[j] = o; }
put body on individual lines
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-780423.js
File test/mjsunit/regress/regress-780423.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-780423.js#newcode31
test/mjsunit/regress/regress-780423.js:31: };
move }; up
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-799761.js
File test/mjsunit/regress/regress-799761.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-799761.js#newcode55
test/mjsunit/regress/regress-799761.js:55: }
else up and add {} for else
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-815.js
File test/mjsunit/regress/regress-815.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-815.js#newcode46
test/mjsunit/regress/regress-815.js:46: o[+o](1,2,3);
spaces after ,
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-85177.js
File test/mjsunit/regress/regress-85177.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-85177.js#newcode30
test/mjsunit/regress/regress-85177.js:30: gW=gH=175;
var?
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-85177.js#newcode31
test/mjsunit/regress/regress-85177.js:31: g=[];
var?
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-85177.js#newcode34
test/mjsunit/regress/regress-85177.js:34: var l=[];
indention
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-85177.js#newcode42
test/mjsunit/regress/regress-85177.js:42: if(a<0||b<0||a>=gW||b>=gH) {
indention
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-85177.js#newcode49
test/mjsunit/regress/regress-85177.js:49: for(var a=[],f=0; f<gW; f++){
indention
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-85177.js#newcode53
test/mjsunit/regress/regress-85177.js:53: for(var i=-1; i<=1; i++)
add {} around for body
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-85177.js#newcode54
test/mjsunit/regress/regress-85177.js:54: for(var j=-1; j<=1; j++)
add {} around for body
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-892742.js
File test/mjsunit/regress/regress-892742.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-892742.js#newcode35
test/mjsunit/regress/regress-892742.js:35: return;/* Counts as
line-terminator whitespace.
according to comment we should not add this here
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-892742.js#newcode40
test/mjsunit/regress/regress-892742.js:40: return;// Comment doesn't
include line-terminator at end.
don't add here
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-931.js
File test/mjsunit/regress/regress-931.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/regress/regress-931.js#newcode33
test/mjsunit/regress/regress-931.js:33: 2: function (x, y) { return x -
y;} };
space after ;
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/simple-constructor.js
File test/mjsunit/simple-constructor.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/simple-constructor.js#newcode126
test/mjsunit/simple-constructor.js:126: this.x=1;
space around =
kmil...@chromium.org
For failing tests, it's sometimes nice to ask the person who last modified
the
code what the code is intended to test.
This makes that person you for a number of tests.
http://codereview.chromium.org/8888006/diff/1/src/array.js
File src/array.js (right):
http://codereview.chromium.org/8888006/diff/1/src/array.js#newcode1
src/array.js:1: // Copyright 2010 the V8 project authors. All rights
reserved.
Here and elsewhere 2011.
da...@chromium.org
disclosure:
I only randomly sampled, I don't have the time to go through all). If such
changes were on the C/C++ code base to ensure maintainability, that would
be one
thing, but the tests rarely get changed, and so I don't see the net added
value
of this enormous CL. Please tell me that I am missing something?
l...@chromium.org
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js
File test/mjsunit/arguments-read-and-assignment.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js#newcode74
test/mjsunit/arguments-read-and-assignment.js:74:
Feel free. I'll try not to expand the scope of this CL too much.
Also, it's not just functions, but function applications. It's hard
enough to read as is.
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-concat.js
File test/mjsunit/array-concat.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-concat.js#newcode228
test/mjsunit/array-concat.js:228: function mkGetter(i) { return
function() { trace.push(i); }; }
No, it ends the return statement.
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js
File test/mjsunit/debug-backtrace.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode126
test/mjsunit/debug-backtrace.js:126: json =
'{"seq":0,"type":"request","command":"backtrace","arguments":{"fromFrame":1,"toFrame":3}}';
Single string JSON literal. I don't want to start splitting it (and
change it from a sequential string to a cons string).
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode144
test/mjsunit/debug-backtrace.js:144: json =
'{"seq":0,"type":"request","command":"backtrace","arguments":{"fromFrame":0,"toFrame":2,
"bottom":true}}';
ditto.
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode176
test/mjsunit/debug-backtrace.js:176: json =
'{"seq":0,"type":"request","command":"frame","arguments":{"number":0}}';
ditto.
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode190
test/mjsunit/debug-backtrace.js:190: json =
'{"seq":0,"type":"request","command":"frame","arguments":{"number":1}}';
ditto
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode201
test/mjsunit/debug-backtrace.js:201: json =
'{"seq":0,"type":"request","command":"frame","arguments":{"number":3}}';
ditto
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-breakpoints.js
File test/mjsunit/debug-breakpoints.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-breakpoints.js#newcode32
test/mjsunit/debug-breakpoints.js:32: function f() {a=1;b=2;}
Actual function body is being used in tests below. Better leave it be.
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-breakpoints.js#newcode37
test/mjsunit/debug-breakpoints.js:37:
Again the missing space is because it refers to the actual body above.
I'm not about to rewrite the the checks of the test in this CL.
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/html-comments.js
File test/mjsunit/html-comments.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/html-comments.js#newcode47
test/mjsunit/html-comments.js:47: var x = 1; x <! x--;
Actually, it shouldn't be above either. This test checks that <!--
begins a comment, and isn't an operator, so it relies on ASI.
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/no-semicolon.js
File test/mjsunit/no-semicolon.js (right):
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/no-semicolon.js#newcode30
test/mjsunit/no-semicolon.js:30:
Whoops, agree.
l...@chromium.org
bring our existing code closer to the coding standard that new code should
use
(and the old code just shows that we haven't been that good at sticking to
so
far).
It should also have the advantage of being a good example for new code - if
new
authors look at old code to see how to write it, the old code should be a
good
example.
The tests might not be worth it though.
Maybe I should just restrict the fixups to the internal javascript files
(src/*.js)? These are being actively developed, where the tests are
write-once
and only look back if they fail.
Kevin Millikin
I'm all for making them clean going forward: cleanup if they're edited, and instructions to developers and reviewers to also pay attention to style in tests.