Revision: 5638
Author: [email protected]
Date: Mon Nov 25 20:27:00 2013 UTC
Log: Scanner: more information about function calls in program output.
https://codereview.appspot.com/31170043
When the scanner prints a problem it prints an expression which, ideally
can be evaluated to obtain the problematic object. These expressions had
omitted function argument and 'this' values; now they are present,
though not yet always evaluatable (primitives are supported, but not
objects and functions). Also, try/catch blocks are emitted instead of
the hypothetical "throws" function.
In order to handle 'this' cleanly, each expression generator now also
returns what the this-value would be if the expression were called.
This is used to decide whether .call() is needed.
Future work will be to arrange for more objects to be printed usefully.
One thing that comes to mind is to have a 'global' WeakMap which maps
references that came from e.g. obtainInstance to appropriate
expressions -- rather than adding Contexts to each argument/this.
This is a partial fix for issue 1753,
https://code.google.com/p/google-caja/issues/detail?id=1753
[email protected]
http://code.google.com/p/google-caja/source/detail?r=5638
Modified:
/trunk/tests/com/google/caja/plugin/test-scan-core.js
/trunk/tests/com/google/caja/plugin/test-scan-guest.html
/trunk/tests/com/google/caja/plugin/test-scan-guest.js
=======================================
--- /trunk/tests/com/google/caja/plugin/test-scan-core.js Wed Nov 6
02:18:47 2013 UTC
+++ /trunk/tests/com/google/caja/plugin/test-scan-core.js Mon Nov 25
20:27:00 2013 UTC
@@ -174,12 +174,33 @@
}
}
- function printNamedly(object) {
- if (typeof object === 'function') {
- var name = getFunctionName(object);
- return name === '' ? object.toString().replace(/\s+/, ' ') : name;
+ function printNamedly(value) {
+ if (typeof value === 'function') {
+ var name = getFunctionName(value);
+ return name === '' ? value.toString().replace(/\s+/, ' ') : name;
} else {
- return String(object);
+ return String(value);
+ }
+ }
+
+ function printEvaluatably(value) {
+ switch (typeof value) {
+ case 'number':
+ if (isNaN(value)) {
+ return 'NaN';
+ } else if (!isFinite(value) && value > 0) {
+ return 'Infinity';
+ } else if (!isFinite(value) && value < 0) {
+ return '-Infinity';
+ } else {
+ return value.toString();
+ }
+ case 'object':
+ case 'function':
+ // TODO(kpreid): implement this where feasible
+ return '.../* ' + printNamedly(value) + ' */';
+ default:
+ return JSON.stringify(value);
}
}
@@ -395,13 +416,17 @@
* object on methods (i.e. an instance if the object is a
prototype).
* @param getThisArgC Ditto but for the thisArg for invoking the object
* itself as a function/method.
+ * @param getProgramInfo Returns { src:, thisArg: } where 'src' is an
+ * expression to return the object and 'thisArg' is the object
which
+ * would be the 'this' value in a function call expression of the
form
+ * (src + '(' + ... + ')').
*
* getSelfC and getThisArgC are thunks (lazy) so that we can avoid
* attempting to obtain an instance of a
not-actually-a-constructor.
* @param getProgram A thunk for program source which returns the
object.
*/
function makeContext(object, path, depth, getSelfC, getThisArgC,
- getProgram) {
+ getProgramInfo) {
var context;
var prefix = path === '' ? '' : path + '.';
if (getSelfC === 'self') {
@@ -415,20 +440,24 @@
toDetailsString: function() {
return (
'| Path: ' + path + '\n' +
- '| Program: ' + getProgram() + '\n' +
+ '| Program: ' + context.getProgram() + '\n' +
'| toString: ' + safeToString(object));
},
get: function() { return object; },
getPath: function() { return path; },
getDepth: function() { return depth; },
- getter: function(name, getter) {
+ getter: function(name, desc, getter) {
function getterProgram() {
- return 'Object.getOwnPropertyDescriptor(' +
- context.getProgram() + ', "' + name + '").get';
+ return {
+ thisArg: desc,
+ src: 'Object.getOwnPropertyDescriptor(' +
+ context.getProgram() + ', "' + name + '").get'
+ };
}
getterProgram.invocationShortcut = function() {
return context.getProgram() + '.' + name;
};
+ getterProgram.invocationShortcut.implicitThis = context.get();
return makeContext(
getter,
prefix + 'get ' + name,
@@ -437,7 +466,7 @@
getSelfC,
getterProgram);
},
- setter: function(name, setter) {
+ setter: function(name, desc, setter) {
return makeContext(
setter,
prefix + 'set ' + name,
@@ -445,8 +474,11 @@
'self',
getSelfC,
function() {
- return 'Object.getOwnPropertyDescriptor(' +
- context.getProgram() + ', "' + name + '").set';
+ return {
+ thisArg: desc,
+ src: 'Object.getOwnPropertyDescriptor(' +
+ context.getProgram() + ', "' + name + '").set'
+ };
});
},
property: function(p, pval, obtainInstance) {
@@ -471,7 +503,10 @@
},
getSelfC,
function() {
- return context.getProgram() + '.prototype';
+ return {
+ thisArg: pval,
+ src: context.getProgram() + '.prototype'
+ };
});
return protoctx;
} else if (p === '[[Prototype]]') {
@@ -482,7 +517,10 @@
getSelfC,
function() { return noThisContext; },
function() {
- return 'Object.getPrototypeOf(' + context.getProgram()
+ ')';
+ return {
+ thisArg: pval,
+ src: 'Object.getPrototypeOf(' + context.getProgram()
+ ')'
+ };
});
} else {
return makeContext(
@@ -492,37 +530,71 @@
'self',
getSelfC,
function() {
+ var src;
if (/^[a-z_]\w*$/i.test(p)) {
- return context.getProgram() + '.' + p;
- } else if (/^-?[0-9]+$/.test(p)) {
- p = parseInt(p);
+ src = context.getProgram() + '.' + p;
+ } else {
+ if (/^-?[0-9]+$/.test(p)) {
+ p = parseInt(p);
+ }
+ src = context.getProgram() + '[' + JSON.stringify(p)
+ ']';
}
- return context.getProgram() + '[' + JSON.stringify(p)
+ ']';
+ return {
+ thisArg: pval,
+ src: src
+ };
});
}
},
- invocation: function(ival, argstr, thrown) {
+ invocation: function(ival, thisArgSpec, args, thrown) {
+ var thisArgStr;
+ switch (thisArgSpec) {
+ case CONSTRUCT: thisArgStr = 'CONSTRUCT'; break;
+ case THIS: thisArgStr = 'THIS'; break;
+ case PLAIN_CALL: thisArgStr = 'PLAIN'; break;
+ default: thisArgStr = String(thisArgSpec); break;
+ }
+ var callStr = '<' + thisArgStr + '>(' + args.map(printNamedly)
+ ')';
return makeContext(
ival,
- path + argstr + (thrown ? ' thrown ' : ''),
+ path + callStr + (thrown ? ' thrown ' : ''),
depth + 1,
'self',
getSelfC,
function() {
- if (thrown) {
- return 'thrown(' + context.getProgram() +
- ', ...)';
- } else if ('invocationShortcut' in context.getProgram) {
- // TODO(kpreid): check args once that is formalized
- return context.getProgram.invocationShortcut();
+ var argsStr = args.map(printEvaluatably);
+ var functionInfo = getProgramInfo();
+ var program;
+ if ('invocationShortcut' in getProgramInfo &&
+ thisArgSpec === THIS &&
+ context.getThisArg() ===
+ getProgramInfo.invocationShortcut.implicitThis &&
+ args.length === 0) {
+ program = getProgramInfo.invocationShortcut();
+ } else if (thisArgSpec === CONSTRUCT) {
+ program = 'new (' + functionInfo.src + ')(' + argsStr
+ ')';
+ } else if (thisArgSpec === THIS &&
+ context.getThisArg() === functionInfo.thisArg) {
+ program = functionInfo.src + '(' + argsStr + ')';
+ } else if (thisArgSpec === PLAIN_CALL) {
+ program = '(0,' + functionInfo.src + ')(' + argsStr
+ ')';
} else {
- return context.getProgram() + '.call(...)';
+ program = functionInfo.src + '.call(' +
+ printEvaluatably(thisArgSpec) + ', ' + argsStr + ')';
}
+ if (thrown) {
+ program = '(function(){try{ ' + program +
+ ' }catch(e){return e}}())';
+ }
+ return {
+ thisArg: undefined,
+ src: program
+ };
});
},
getThisArg: function() { return getThisArgC().get(); },
/** Return a program which evaluates to the object. */
- getProgram: getProgram
+ getProgram: function() { return getProgramInfo().src; }
};
return context;
}
@@ -532,7 +604,7 @@
'self',
0,
function() { return noThisContext; },
- function() { return 'undefined'; });
+ function() { return { thisArg: undefined, src: 'undefined' }; });
return {
root: function(o, path, code) {
@@ -542,7 +614,7 @@
0,
'self',
function() { return noThisContext; },
- function() { return code; });
+ function() { return { thisArg: undefined, src: code }; });
}
};
})();
@@ -700,7 +772,8 @@
var didPlainCall = false;
var doInvocation = function(tuple) {
- var thisArg = tuple[0];
+ var thisArgSpec = tuple[0];
+ var thisArg = thisArgSpec;
var args = tuple[1];
var hook = tuple[2];
if (tuple.length < 2 || tuple.length > 3 ||
@@ -710,10 +783,8 @@
'\n' + context.toDetailsString());
}
var result;
- var thisArgStr = '<' + thisArg + '>';
var thrown;
if (thisArg === CONSTRUCT) {
- thisArgStr = '<CONSTRUCT>';
try {
switch (args.length) {
case 0: result = new object(); break;
@@ -734,7 +805,6 @@
thrown = true;
}
} else if (thisArg === PLAIN_CALL) {
- thisArgStr = '<PLAIN>';
didPlainCall = true;
// Do a plain function call, literally, with no call/apply
try {
@@ -747,7 +817,6 @@
} else {
if (thisArg === THIS) {
thisArg = context.getThisArg();
- thisArgStr = '<THIS>';
}
try {
result = Function.prototype.apply.call(object, thisArg,
args);
@@ -759,10 +828,9 @@
}
didSomeCall = true;
didNonThrowingCall = didNonThrowingCall || !thrown;
- // TODO(kpreid): Make these live objects instead of strings,
so that
- // we can print clearer paths and more complete programs.
- var subcontext = context.invocation(result, thisArgStr + '(' +
- args.map(printNamedly) + ')', thrown);
+ // TODO(kpreid): Should pass in real thisArg, maybe in a
context
+ var subcontext = context.invocation(
+ result, thisArgSpec, args, thrown);
if (hook) {
// TODO(kpreid): Context should include thrown flag
hook(subcontext, thrown);
@@ -841,10 +909,10 @@
queue(context.property(name, v, obtainInstance), last);
}
if ((v = desc.get)) {
- queue(context.getter(name, v), last);
+ queue(context.getter(name, desc, v), last);
}
if ((v = desc.set)) {
- queue(context.setter(name, v), last);
+ queue(context.setter(name, desc, v), last);
}
}
var ownPropertyNames;
=======================================
--- /trunk/tests/com/google/caja/plugin/test-scan-guest.html Fri Aug 23
17:58:39 2013 UTC
+++ /trunk/tests/com/google/caja/plugin/test-scan-guest.html Mon Nov 25
20:27:00 2013 UTC
@@ -36,13 +36,11 @@
}
</style>
<body>
+
<script src="test-scan-core.js"></script>
-<script src="test-scan-guest.js"></script>
-<div class="testcontainer" id="testScanner"></div>
-<script>
- jsunitRegister('testScanner', testScanner);
-</script>
+<!-- test-scan-guest will register its self-tests when loaded -->
+<script src="test-scan-guest.js"></script>
<form>
<input type="text" id="scan-only" value="" size="100">
=======================================
--- /trunk/tests/com/google/caja/plugin/test-scan-guest.js Tue Nov 12
18:26:59 2013 UTC
+++ /trunk/tests/com/google/caja/plugin/test-scan-guest.js Mon Nov 25
20:27:00 2013 UTC
@@ -23,7 +23,8 @@
* setTimeout, clearTimeout, setInterval, clearInterval,
* requestAnimationFrame, cancelAnimationFrame,
* cajaVM, directAccess, getUrlParam,
- * assertTrue, assertEquals, pass, jsunitFail,
+ * assertTrue, assertEquals, jsunitRegister, jsunitCallback, pass,
+ * jsunitFail,
* Event, HTMLInputElement, HTMLMediaElement, HTMLTableRowElement,
* HTMLTableSectionElement, HTMLTableElement, HTMLImageElement,
* HTMLTextAreaElement, HTMLVideoElement, HTMLButtonElement,
@@ -194,7 +195,6 @@
scanner.skip('scanning');
scanner.skip('toggleNonErrors');
scanner.skip('testUniverse');
- scanner.skip('testScanner');
// returns its parameters to help catch leaks
function dummyFunction() {
@@ -1451,7 +1451,9 @@
}
};
- // used by testScanner only
+ // --- Tests of the scanner's own functionality. ---
+
+ // used by meta tests only
/** Make a simple isolated object graph. */
function detach(object, seen) {
if (object !== Object(object)) { return object; }
@@ -1473,18 +1475,18 @@
return copy;
}
- /** Test the scanner's own functionality. */
- window.testScanner = function testScanner() {
- // non-identifier props in programs
+ jsunitRegister('testMetaScannerAndProgramIdentifiers', function() {
+ // Testing the scanner operation, and incidentally that property names
+ // are correctly handled.
var problemPrograms = ['bar["!"]', 'bar[1]', 'bar.a'];
var scanner = new Scanner({
logProblem: function(description, context) {
assertEquals(problemPrograms.shift(), context.getProgram());
},
- finished: function() {
+ finished: jsunitCallback(function() {
assertEquals(0, problemPrograms.length);
- pass('testScanner');
- }
+ pass();
+ })
});
// each property here is an 'object is extensible' problem
@@ -1492,7 +1494,37 @@
scanner.queue(Context.root(specimen, 'foo', 'bar'));
scanner.scan();
+ });
+ jsunitRegister('testMetaPrograms', function() {
+ // TODO(kpreid): Would be better if there was an abstraction over
performing
+ // the operations (that is, something which e.g. both invokes a method
and
+ // constructs the Context for its return value), so we don't have to
use the
+ // Context operations which are kinda internal.
+ var c = Context.root(function f() {}, 'foo', 'bar');
+ assertEquals('root program', 'bar', c.getProgram());
+ assertEquals('proto', 'Object.getPrototypeOf(bar)',
+ c.property('[[Prototype]]', 'junk', null).getProgram());
+
+ assertEquals('invocation not thrown', 'bar.call("thiss", "arg1")',
+ c.invocation(2, 'thiss', ['arg1'], false).getProgram());
+ assertEquals('invocation thrown',
+ '(function(){try{ bar.call("thiss", "arg1") }catch(e){return
e}}())',
+ c.invocation(2, 'thiss', ['arg1'], true).getProgram());
+ assertEquals('constructor', 'new (bar)()',
+ c.invocation(2, CONSTRUCT, [], false).getProgram());
+ // TODO(kpreid): unrealistic values
+ assertEquals('method call', 'bar("arg1")',
+ c.invocation(2, THIS, ['arg1'], false).getProgram());
+ assertEquals('plain call', '(0,bar)("arg1")',
+ c.invocation(2, PLAIN_CALL, ['arg1'], false).getProgram());
+
+ // TODO(kpreid): test (and implement) argument lists with object values
+
+ pass();
+ });
+
+ jsunitRegister('testMetaTrueApply', function() {
// test trueApply which has proven to be tricky.
var trueApply = scanning.trueApply;
function argsIdentity() {
@@ -1502,9 +1534,12 @@
assertEquals('1 a', trueApply(argsIdentity, ['a']));
assertEquals('2 a,b', trueApply(argsIdentity, ['a', 'b']));
assertEquals('3 a,b,c', trueApply(argsIdentity, ['a', 'b', 'c']));
+ pass();
+ });
- // TODO(kpreid): more meta-tests
- };
+ // TODO(kpreid): more meta-tests
+
+ // --- UI glue ---
// wire up checkbox
var hideCheckbox;
--
---
You received this message because you are subscribed to the Google Groups "Google Caja Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
