Sub-class caja runtime exceptions so they are easier to track and distinguish

Creates a new subclass of RuntimeException which the cajoler throws
when there it can no longer proceed.
Added explanations for a number of these exceptions

@3912

[MikeSamuel, 2009-10-14 05:03:25]

http://codereview.appspot.com/129056/diff/1/35
File src/com/google/caja/CajaException.java (right):

http://codereview.appspot.com/129056/diff/1/35#newcode1
Line 1: // Copyright (C) 2009 Google Inc.
why?

http://codereview.appspot.com/129056/diff/1/46
File src/com/google/caja/cajita.js (right):

http://codereview.appspot.com/129056/diff/1/46#newcode1573
Line 1573: }
What does this have to do with runtime exceptions?

http://codereview.appspot.com/129056/diff/1/47
File src/com/google/caja/demos/applet/CajaApplet.java (right):

http://codereview.appspot.com/129056/diff/1/47#newcode17
Line 17: import com.google.caja.SomethingWidgyHappenedError;
Has this file been svn added?

http://codereview.appspot.com/129056/diff/1/48
File src/com/google/caja/demos/applet/index.html (right):

http://codereview.appspot.com/129056/diff/1/48#newcode298
Line 298: -->
?

http://codereview.appspot.com/129056/diff/1/16
File src/com/google/caja/lexer/escaping/Escaping.java (right):

http://codereview.appspot.com/129056/diff/1/16#newcode62
Line 62: "StringBuilders don't throw IOException", ex);
indent 4

http://codereview.appspot.com/129056/diff/1/29
File src/com/google/caja/parser/html/CajaTreeBuilder.java (right):

http://codereview.appspot.com/129056/diff/1/29#newcode121
Line 121: "Unexpected parsing error", ex);
indent

http://codereview.appspot.com/129056/diff/1/50
File src/com/google/caja/valija-cajita.js (right):

http://codereview.appspot.com/129056/diff/1/50#newcode324
Line 324: var template = cajita.PseudoFunction(callFn);
unrelated

http://codereview.appspot.com/129056/diff/1/5
File tests/com/google/caja/parser/quasiliteral/CommonJsRewriterTestCase.java
(right):

http://codereview.appspot.com/129056/diff/1/5#newcode419
Line 419: rewriteAndExecute("assertTrue(''+JSON === '[object Object]');");
unrelated

http://codereview.appspot.com/129056/diff/1/2
File tests/com/google/caja/render/OrigSourceRendererTestCase.java (right):

http://codereview.appspot.com/129056/diff/1/2#newcode126
Line 126: "No sources specified");
indent

[Jasvir, 2009-10-27 06:00:15]

Updated and snapshot.

http://codereview.appspot.com/129056/diff/1/35
File src/com/google/caja/CajaException.java (right):

http://codereview.appspot.com/129056/diff/1/35#newcode1
Line 1: // Copyright (C) 2009 Google Inc.
On 2009/10/14 05:03:25, MikeSamuel wrote:
> why?

Done.

http://codereview.appspot.com/129056/diff/1/46
File src/com/google/caja/cajita.js (right):

http://codereview.appspot.com/129056/diff/1/46#newcode1573
Line 1573: }
Something widgey happened mixed up two clients.
Removed.

On 2009/10/14 05:03:25, MikeSamuel wrote:
> What does this have to do with runtime exceptions?

http://codereview.appspot.com/129056/diff/1/47
File src/com/google/caja/demos/applet/CajaApplet.java (right):

http://codereview.appspot.com/129056/diff/1/47#newcode17
Line 17: import com.google.caja.SomethingWidgyHappenedError;
On 2009/10/14 05:03:25, MikeSamuel wrote:
> Has this file been svn added?

Done.

http://codereview.appspot.com/129056/diff/1/48
File src/com/google/caja/demos/applet/index.html (right):

http://codereview.appspot.com/129056/diff/1/48#newcode298
Line 298: -->
On 2009/10/14 05:03:25, MikeSamuel wrote:
> ?

Done.

http://codereview.appspot.com/129056/diff/1/16
File src/com/google/caja/lexer/escaping/Escaping.java (right):

http://codereview.appspot.com/129056/diff/1/16#newcode62
Line 62: "StringBuilders don't throw IOException", ex);
On 2009/10/14 05:03:25, MikeSamuel wrote:
> indent 4

Done.

http://codereview.appspot.com/129056/diff/1/29
File src/com/google/caja/parser/html/CajaTreeBuilder.java (right):

http://codereview.appspot.com/129056/diff/1/29#newcode121
Line 121: "Unexpected parsing error", ex);
On 2009/10/14 05:03:25, MikeSamuel wrote:
> indent

Done.

http://codereview.appspot.com/129056/diff/1/50
File src/com/google/caja/valija-cajita.js (right):

http://codereview.appspot.com/129056/diff/1/50#newcode324
Line 324: var template = cajita.PseudoFunction(callFn);
On 2009/10/14 05:03:25, MikeSamuel wrote:
> unrelated

Done.

http://codereview.appspot.com/129056/diff/1/5
File tests/com/google/caja/parser/quasiliteral/CommonJsRewriterTestCase.java
(right):

http://codereview.appspot.com/129056/diff/1/5#newcode419
Line 419: rewriteAndExecute("assertTrue(''+JSON === '[object Object]');");
On 2009/10/14 05:03:25, MikeSamuel wrote:
> unrelated

Done.

http://codereview.appspot.com/129056/diff/1/2
File tests/com/google/caja/render/OrigSourceRendererTestCase.java (right):

http://codereview.appspot.com/129056/diff/1/2#newcode126
Line 126: "No sources specified");
On 2009/10/14 05:03:25, MikeSamuel wrote:
> indent

Done.

[MikeSamuel, 2009-11-11 20:49:01]

Should AssertionErrors also be replaced?  I, at least, have used them
inconsistently where I otherwise used RuntimeException.

> find src -name \*.java | xargs grep -n 'new AssertionError' | perl -pe
's|^src/com/google/[a-z/]+/||'
ConfigUtil.java:535:      throw new
AssertionError(obj.getClass().getSimpleName());
HtmlLexer.java:680:                  throw new AssertionError();
NumberRecognizer.java:63:          throw new AssertionError();
AbstractParseTreeNode.java:225:      throw new AssertionError("StringBuilders
shouldn't throw IOExceptions");
AbstractParseTreeNode.java:237:      throw new AssertionError("StringBuilders
shouldn't throw IOExceptions");
CssTree.java:72:      throw new AssertionError("StringBuilders shouldn't throw
IOExceptions");
NoChildren.java:29:  public Object getValue() { throw new AssertionError(); }
NoChildren.java:30:  public void render(RenderContext rc) { throw new
AssertionError(); }
NoChildren.java:31:  public String typeOf() { throw new AssertionError(); }
Operator.java:214:          throw new AssertionError("Duplicate symbol " +
op.getSymbol()
Operator.java:227:        throw new AssertionError(
Parser.java:502:        throw new AssertionError(t.text);
Parser.java:913:            throw new AssertionError();
Rewriter.java:92:            throw new AssertionError(
CssRewriter.java:712:                throw new AssertionError();
CssValidator.java:804:        throw new
AssertionError(sig.getClass().getName());
CssValidator.java:1035:      throw new AssertionError(
CssValidator.java:1357:      throw new AssertionError(
PluginCompilerMain.java:209:      throw new AssertionError("Can't classify input
" + is);
PluginCompilerMain.java:267:        throw new AssertionError(
PluginCompilerMain.java:332:      throw new AssertionError(e);
TemplateSanitizer.java:121:        throw new AssertionError(t.getNodeName());
Message.java:77:      AssertionError e = new AssertionError(
RhinoExecutor.java:499:            throw (AssertionError) new AssertionError(

http://codereview.appspot.com/129056/diff/2001/3046
File src/com/google/caja/SomethingWidgyHappenedError.java (right):

http://codereview.appspot.com/129056/diff/2001/3046#newcode19
src/com/google/caja/SomethingWidgyHappenedError.java:19: /**
Plz add blank line after imports.

http://codereview.appspot.com/129056/diff/2001/3046#newcode37
src/com/google/caja/SomethingWidgyHappenedError.java:37: super(msg.format(new
MessageContext()), cause);
ok

http://codereview.appspot.com/129056/diff/2001/3048
File src/com/google/caja/demos/applet/CajaApplet.java (right):

http://codereview.appspot.com/129056/diff/2001/3048#newcode129
src/com/google/caja/demos/applet/CajaApplet.java:129: throw new
SomethingWidgyHappenedError("UTF-8 should be supported",
Plz split the line after (

The ways of formatting from the C++ guidelines are
  foobar(baz,
         boo);
or break after ( and indent by 4
  foobar(
      baz, boo);

http://codereview.appspot.com/129056/diff/2001/3017
File src/com/google/caja/lexer/escaping/Escaping.java (right):

http://codereview.appspot.com/129056/diff/2001/3017#newcode166
src/com/google/caja/lexer/escaping/Escaping.java:166: "StringBuilders don't
throw IOException");
indent

http://codereview.appspot.com/129056/diff/2001/3017#newcode194
src/com/google/caja/lexer/escaping/Escaping.java:194: "StringBuilders don't
throw IOException");
ditto

http://codereview.appspot.com/129056/diff/2001/3017#newcode532
src/com/google/caja/lexer/escaping/Escaping.java:532: "StringBuilders don't
throw IOException");
ditto

http://codereview.appspot.com/129056/diff/2001/3031
File src/com/google/caja/parser/html/CajaTreeBuilder.java (right):

http://codereview.appspot.com/129056/diff/2001/3031#newcode121
src/com/google/caja/parser/html/CajaTreeBuilder.java:121: "Unexpected parsing
error", ex);
indent

http://codereview.appspot.com/129056/diff/2001/3042
File src/com/google/caja/plugin/templates/Localizer.java (right):

http://codereview.appspot.com/129056/diff/2001/3042#newcode228
src/com/google/caja/plugin/templates/Localizer.java:228: "IOException reading
from String");
Add ex as cause plz.

http://codereview.appspot.com/129056/diff/2001/3002
File tests/com/google/caja/lexer/JsLexerTest.java (right):

http://codereview.appspot.com/129056/diff/2001/3002#newcode18
tests/com/google/caja/lexer/JsLexerTest.java:18: import
com.google.caja.util.TestUtil;
unnecessary imports

http://codereview.appspot.com/129056/diff/2001/3010
File tests/com/google/caja/plugin/templates/TemplateCompilerTest.java (right):

http://codereview.appspot.com/129056/diff/2001/3010#newcode66
tests/com/google/caja/plugin/templates/TemplateCompilerTest.java:66:
MessagePart.Factory.valueOf(
indent

http://codereview.appspot.com/129056/diff/2001/3010#newcode357
tests/com/google/caja/plugin/templates/TemplateCompilerTest.java:357:
MessagePart.Factory.valueOf(
ditto

http://codereview.appspot.com/129056/diff/2001/3015
File tests/com/google/caja/util/CajaTestCase.java (right):

http://codereview.appspot.com/129056/diff/2001/3015#newcode260
tests/com/google/caja/util/CajaTestCase.java:260: throw new
SomethingWidgyHappenedError("IOException from StringBuilder");
Please thread ex through.

[Jasvir, 2009-12-01 23:40:42]

Snapshot.

Also replaced assertion errors.

http://codereview.appspot.com/129056/diff/2001/3046
File src/com/google/caja/SomethingWidgyHappenedError.java (right):

http://codereview.appspot.com/129056/diff/2001/3046#newcode19
src/com/google/caja/SomethingWidgyHappenedError.java:19: /**
On 2009/11/11 20:49:01, MikeSamuel wrote:
> Plz add blank line after imports.

Done.

http://codereview.appspot.com/129056/diff/2001/3048
File src/com/google/caja/demos/applet/CajaApplet.java (right):

http://codereview.appspot.com/129056/diff/2001/3048#newcode129
src/com/google/caja/demos/applet/CajaApplet.java:129: throw new
SomethingWidgyHappenedError("UTF-8 should be supported",
On 2009/11/11 20:49:01, MikeSamuel wrote:
> Plz split the line after (
> 
> The ways of formatting from the C++ guidelines are
>   foobar(baz,
>          boo);
> or break after ( and indent by 4
>   foobar(
>       baz, boo);
> 

Done.

http://codereview.appspot.com/129056/diff/2001/3017
File src/com/google/caja/lexer/escaping/Escaping.java (right):

http://codereview.appspot.com/129056/diff/2001/3017#newcode166
src/com/google/caja/lexer/escaping/Escaping.java:166: "StringBuilders don't
throw IOException");
On 2009/11/11 20:49:01, MikeSamuel wrote:
> indent

Done.

http://codereview.appspot.com/129056/diff/2001/3017#newcode194
src/com/google/caja/lexer/escaping/Escaping.java:194: "StringBuilders don't
throw IOException");
On 2009/11/11 20:49:01, MikeSamuel wrote:
> ditto

Done.

http://codereview.appspot.com/129056/diff/2001/3017#newcode532
src/com/google/caja/lexer/escaping/Escaping.java:532: "StringBuilders don't
throw IOException");
On 2009/11/11 20:49:01, MikeSamuel wrote:
> ditto

Done.

http://codereview.appspot.com/129056/diff/2001/3042
File src/com/google/caja/plugin/templates/Localizer.java (right):

http://codereview.appspot.com/129056/diff/2001/3042#newcode228
src/com/google/caja/plugin/templates/Localizer.java:228: "IOException reading
from String");
On 2009/11/11 20:49:01, MikeSamuel wrote:
> Add ex as cause plz.

Done.

http://codereview.appspot.com/129056/diff/2001/3002
File tests/com/google/caja/lexer/JsLexerTest.java (right):

http://codereview.appspot.com/129056/diff/2001/3002#newcode18
tests/com/google/caja/lexer/JsLexerTest.java:18: import
com.google.caja.util.TestUtil;
On 2009/11/11 20:49:01, MikeSamuel wrote:
> unnecessary imports

Done.

http://codereview.appspot.com/129056/diff/2001/3010
File tests/com/google/caja/plugin/templates/TemplateCompilerTest.java (right):

http://codereview.appspot.com/129056/diff/2001/3010#newcode66
tests/com/google/caja/plugin/templates/TemplateCompilerTest.java:66:
MessagePart.Factory.valueOf(
On 2009/11/11 20:49:01, MikeSamuel wrote:
> indent

Done.

http://codereview.appspot.com/129056/diff/2001/3010#newcode357
tests/com/google/caja/plugin/templates/TemplateCompilerTest.java:357:
MessagePart.Factory.valueOf(
On 2009/11/11 20:49:01, MikeSamuel wrote:
> ditto

Done.

http://codereview.appspot.com/129056/diff/2001/3015
File tests/com/google/caja/util/CajaTestCase.java (right):

http://codereview.appspot.com/129056/diff/2001/3015#newcode260
tests/com/google/caja/util/CajaTestCase.java:260: throw new
SomethingWidgyHappenedError("IOException from StringBuilder");
On 2009/11/11 20:49:01, MikeSamuel wrote:
> Please thread ex through.

Done.

[MikeSamuel, 2009-12-03 01:57:40]

LGTM with changes below.

So we should use SomethingWidgyHappened* in regular code so that people looking
through logs know that the problem originated with Caja code.

But that's not as important for test code, right?

http://codereview.appspot.com/129056/diff/6054/7020
File src/com/google/caja/lexer/escaping/Escaping.java (right):

http://codereview.appspot.com/129056/diff/6054/7020#newcode92
src/com/google/caja/lexer/escaping/Escaping.java:92: "StringBuilders don't throw
IOException");
, ex

http://codereview.appspot.com/129056/diff/6054/7020#newcode129
src/com/google/caja/lexer/escaping/Escaping.java:129: "StringBuilders don't
throw IOException");
, ex

http://codereview.appspot.com/129056/diff/6054/7020#newcode166
src/com/google/caja/lexer/escaping/Escaping.java:166: "StringBuilders don't
throw IOException");
indent
, ex

http://codereview.appspot.com/129056/diff/6054/7020#newcode194
src/com/google/caja/lexer/escaping/Escaping.java:194: "StringBuilders don't
throw IOException");
, ex

http://codereview.appspot.com/129056/diff/6054/7020#newcode216
src/com/google/caja/lexer/escaping/Escaping.java:216: "StringBuilders don't
throw IOException");
, ex

http://codereview.appspot.com/129056/diff/6054/7020#newcode245
src/com/google/caja/lexer/escaping/Escaping.java:245: "StringBuilders don't
throw IOException");
, ex

http://codereview.appspot.com/129056/diff/6054/7020#newcode532
src/com/google/caja/lexer/escaping/Escaping.java:532: "StringBuilders don't
throw IOException");
, ex

http://codereview.appspot.com/129056/diff/6054/7035
File src/com/google/caja/parser/Normalizer.java (right):

http://codereview.appspot.com/129056/diff/6054/7035#newcode78
src/com/google/caja/parser/Normalizer.java:78: "Normalizer unexpectedly
uninvokable", (RuntimeException) th);
Can't we just propagate th up here and in the else clause?
If not, we don't need to branch.

http://codereview.appspot.com/129056/diff/6054/7064
File src/com/google/caja/reporting/Message.java (right):

http://codereview.appspot.com/129056/diff/6054/7064#newcode82
src/com/google/caja/reporting/Message.java:82: throw e;
These three lines can be condensed to 1.

[Jasvir, 2009-12-11 01:28:19]

Snapshot.

http://codereview.appspot.com/129056/diff/6054/7020
File src/com/google/caja/lexer/escaping/Escaping.java (right):

http://codereview.appspot.com/129056/diff/6054/7020#newcode92
src/com/google/caja/lexer/escaping/Escaping.java:92: "StringBuilders don't throw
IOException");
On 2009/12/03 01:57:40, MikeSamuel wrote:
> , ex

Done. Ditto below.

http://codereview.appspot.com/129056/diff/6054/7035
File src/com/google/caja/parser/Normalizer.java (right):

http://codereview.appspot.com/129056/diff/6054/7035#newcode78
src/com/google/caja/parser/Normalizer.java:78: "Normalizer unexpectedly
uninvokable", (RuntimeException) th);
On 2009/12/03 01:57:40, MikeSamuel wrote:
> Can't we just propagate th up here and in the else clause?
> If not, we don't need to branch.

Done.

http://codereview.appspot.com/129056/diff/6054/7064
File src/com/google/caja/reporting/Message.java (right):

http://codereview.appspot.com/129056/diff/6054/7064#newcode82
src/com/google/caja/reporting/Message.java:82: throw e;
On 2009/12/03 01:57:40, MikeSamuel wrote:
> These three lines can be condensed to 1.

Done.

[Jasvir, 2009-12-11 01:29:12]

On 2009/12/03 01:57:40, MikeSamuel wrote:
> LGTM with changes below.
> 
> So we should use SomethingWidgyHappened* in regular code so that people
looking
> through logs know that the problem originated with Caja code.
> 
> But that's not as important for test code, right?

Right.  I got a bit carried away but its definitely not necessary for tests/.

> 
> http://codereview.appspot.com/129056/diff/6054/7020
> File src/com/google/caja/lexer/escaping/Escaping.java (right):
> 
> http://codereview.appspot.com/129056/diff/6054/7020#newcode92
> src/com/google/caja/lexer/escaping/Escaping.java:92: "StringBuilders don't
throw
> IOException");
> , ex
> 
> http://codereview.appspot.com/129056/diff/6054/7020#newcode129
> src/com/google/caja/lexer/escaping/Escaping.java:129: "StringBuilders don't
> throw IOException");
> , ex
> 
> http://codereview.appspot.com/129056/diff/6054/7020#newcode166
> src/com/google/caja/lexer/escaping/Escaping.java:166: "StringBuilders don't
> throw IOException");
> indent
> , ex
> 
> http://codereview.appspot.com/129056/diff/6054/7020#newcode194
> src/com/google/caja/lexer/escaping/Escaping.java:194: "StringBuilders don't
> throw IOException");
> , ex
> 
> http://codereview.appspot.com/129056/diff/6054/7020#newcode216
> src/com/google/caja/lexer/escaping/Escaping.java:216: "StringBuilders don't
> throw IOException");
> , ex
> 
> http://codereview.appspot.com/129056/diff/6054/7020#newcode245
> src/com/google/caja/lexer/escaping/Escaping.java:245: "StringBuilders don't
> throw IOException");
> , ex
> 
> http://codereview.appspot.com/129056/diff/6054/7020#newcode532
> src/com/google/caja/lexer/escaping/Escaping.java:532: "StringBuilders don't
> throw IOException");
> , ex
> 
> http://codereview.appspot.com/129056/diff/6054/7035
> File src/com/google/caja/parser/Normalizer.java (right):
> 
> http://codereview.appspot.com/129056/diff/6054/7035#newcode78
> src/com/google/caja/parser/Normalizer.java:78: "Normalizer unexpectedly
> uninvokable", (RuntimeException) th);
> Can't we just propagate th up here and in the else clause?
> If not, we don't need to branch.
> 
> http://codereview.appspot.com/129056/diff/6054/7064
> File src/com/google/caja/reporting/Message.java (right):
> 
> http://codereview.appspot.com/129056/diff/6054/7064#newcode82
> src/com/google/caja/reporting/Message.java:82: throw e;
> These three lines can be condensed to 1.

[MikeSamuel, 2009-12-11 04:42:23]

Some indentation nits.

LGTM

http://codereview.appspot.com/129056/diff/8003/8023
File src/com/google/caja/lexer/escaping/Escaping.java (right):

http://codereview.appspot.com/129056/diff/8003/8023#newcode166
src/com/google/caja/lexer/escaping/Escaping.java:166: "StringBuilders don't
throw IOException", ex);
indent

http://codereview.appspot.com/129056/diff/8003/8037
File src/com/google/caja/parser/Normalizer.java (right):

http://codereview.appspot.com/129056/diff/8003/8037#newcode50
src/com/google/caja/parser/Normalizer.java:50: // Use heuristic below.
Please don't use tabs

http://codereview.appspot.com/129056/diff/8003/8037#newcode81
src/com/google/caja/parser/Normalizer.java:81: "Normalizer unexpectedly
uninvokable", th);
Indentation.