amohanta / google-caja

Automatically exported from code.google.com/p/google-caja
0 stars 0 forks source link

Code review: mikesamuel/cleanup-29-Jul-2008 (22 added, 381 removed, 59 changed) #666

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
gvn review mikesamuel/cleanup-29-Jul-2008

*mikesamuel/cleanup-29-Jul-2008@2047 | mikesamuel | 2008-07-29 16:39:06
-0800 (Tue, 29 Jul 2008)

Description:

Fix a bunch of IDE warnings to partially address issue 470.

- Added @Override to overriding methods
- Added comments about unused parameters
- Got rid of unused parameters or useless parameters in Rule where the rule
para\
m is always == this
- Made Pair immutable.
- Got rid of unused class CommentLexer.
- Got rid of unused local variables.

This compiles with no warnings in Eclipse with all warnings on except:
- Unqualified access to instance field
- Undocumented empty block
- Access to a non-accessible member of an enclosing type
- Parameter assignment
- Non-externalized string
- Serializable class without serialVersionUID
- Boxing and unboxing conversions
- Enum type constant not covered on switch
- Potential null pointer access
- Local variable declaration hides another field or variable
- Depreacted API
- Redundant null check  (buggy)
- Unnecessary 'else' statement
- Generic type parameter declared with a final type bound
- Missing '@Deprecated' annotation

Issue 470:       Java code should compile clean, with no compiler warnings

Reported by daw+goo...@taverner.cs.berkeley.edu, Jun 10, 2008

What steps will reproduce the problem?
1. Import Caja into Eclipse as a Java project (note: you will have to
delete the tools/ directory and BuildServiceImplementation; you will have
to manually add all of the .jar's under third_party/ as
third-party/external jars in Eclipse's UI)
2. Let Eclipse build the project
3. Notice the warnings

What is the expected output? What do you see instead?

Expected output: No warnings.  Actual: 31 warnings.

What version of the product are you using? On what operating system?

Eclipse 3.3.2, default settings.
5B5B

Affected Paths:
   M //trunk/src/com/google/caja/config/ConfigUtil.java
   D //trunk/src/com/google/caja/lexer/CommentLexer.java
   M //trunk/src/com/google/caja/lexer/JsTokenQueue.java
   M //trunk/src/com/google/caja/opensocial/DefaultGadgetRewriter.java
   M //trunk/src/com/google/caja/opensocial/GadgetParser.java
   M //trunk/src/com/google/caja/opensocial/service/CajolingServiceMain.java
   M //trunk/src/com/google/caja/parser/ParseTreeNodeContainer.java
   M //trunk/src/com/google/caja/parser/css/CssTree.java
   M //trunk/src/com/google/caja/parser/html/DomParser.java
   M //trunk/src/com/google/caja/parser/js/ArrayConstructor.java
   M //trunk/src/com/google/caja/parser/js/Block.java
   M //trunk/src/com/google/caja/parser/js/BooleanLiteral.java
   M //trunk/src/com/google/caja/parser/js/BreakStmt.java
   M //trunk/src/com/google/caja/parser/js/CaseStmt.java
   M //trunk/src/com/google/caja/parser/js/CatchStmt.java
   M //trunk/src/com/google/caja/parser/js/Conditional.java
   M //trunk/src/com/google/caja/parser/js/ContinueStmt.java
   M //trunk/src/com/google/caja/parser/js/DebuggerStmt.java
   M //trunk/src/com/google/caja/parser/js/Declaration.java
   M //trunk/src/com/google/caja/parser/js/DefaultCaseStmt.java
   M //trunk/src/com/google/caja/parser/js/DoWhileLoop.java
   M //trunk/src/com/google/caja/parser/js/ExpressionStmt.java
   M //trunk/src/com/google/caja/parser/js/FinallyStmt.java
   M //trunk/src/com/google/caja/parser/js/FunctionConstructor.java
   M //trunk/src/com/google/caja/parser/js/IntegerLiteral.java
   M //trunk/src/com/google/caja/parser/js/MultiDeclaration.java
   M //trunk/src/com/google/caja/parser/js/Noop.java
   M //trunk/src/com/google/caja/parser/js/NullLiteral.java
   M //trunk/src/com/google/caja/parser/js/ObjectConstructor.java
   M //trunk/src/com/google/caja/parser/js/Parser.java
   M //trunk/src/com/google/caja/parser/js/RealLiteral.java
   M //trunk/src/com/google/caja/parser/js/Reference.java
   M //trunk/src/com/google/caja/parser/js/RegexpLiteral.java
   M //trunk/src/com/google/caja/parser/js/ReturnStmt.java
   M //trunk/src/com/google/caja/parser/js/StringLiteral.java
   M //trunk/src/com/google/caja/parser/js/ThrowStmt.java
   M //trunk/src/com/google/caja/parser/js/TryStmt.java
   M //trunk/src/com/google/caja/parser/js/UndefinedLiteral.java
   M //trunk/src/com/google/caja/parser/js/WithStmt.java
   M //trunk/src/com/google/caja/parser/quasiliteral/DefaultCajaRewriter.java
   M
//trunk/src/com/google/caja/parser/quasiliteral/MultipleNonemptyQuasiHole.j\
ava
   M //trunk/src/com/google/caja/parser/quasiliteral/QuasiBuilder.java
   M //trunk/src/com/google/caja/parser/quasiliteral/Rule.java
   M //trunk/src/com/google/caja/parser/quasiliteral/RuleDoclet.java
   M //trunk/src/com/google/caja/parser/quasiliteral/Scope.java
   M
//trunk/src/com/google/caja/parser/quasiliteral/SingleOptionalQuasiHole.jav\
a
   M //trunk/src/com/google/caja/parser/quasiliteral/SingleQuasiHole.java
   M
//trunk/src/com/google/caja/parser/quasiliteral/TrailingUnderscoresHole.jav\
a
   M //trunk/src/com/google/caja/plugin/CssTemplate.java
   M //trunk/src/com/google/caja/plugin/DomProcessingEvents.java
   M //trunk/src/com/google/caja/plugin/GxpCompiler.java
   M //trunk/src/com/google/caja/plugin/PluginCompilerMain.java
   M //trunk/src/com/google/caja/render/JsMinimalPrinter.java
   M //trunk/src/com/google/caja/render/SideBySideRenderer.java
   M //trunk/src/com/google/caja/render/TabularSideBySideRenderer.java
   M //trunk/src/com/google/caja/util/Pair.java
   M //trunk/tests/com/google/caja/AllTests.java
   M //trunk/tests/com/google/caja/config/ConfigUtilTest.java
   D //trunk/tests/com/google/caja/lexer/CommentLexerTest.java
   M //trunk/tests/com/google/caja/opensocial/DefaultGadgetRewriterTest.java
   M //trunk/tests/com/google/caja/parser/js/FuzzedParserTest.java
   M //trunk/tests/com/google/caja/parser/quasiliteral/MatchExperiments.java
   M //trunk/tests/com/google/caja/parser/quasiliteral/ScopeTest.java
   M //trunk/tests/com/google/caja/plugin/CssPropertyPatternsTest.java
   M //trunk/tests/com/google/caja/plugin/GxpCompilerTest.java
   M //trunk/tests/com/google/caja/render/SideBySideRendererTest.java

Original issue reported on code.google.com by mikesamuel@gmail.com on 30 Jul 2008 at 12:50

GoogleCodeExporter commented 9 years ago
LGTM

Original comment by metaw...@gmail.com on 1 Aug 2008 at 6:04

GoogleCodeExporter commented 9 years ago

Original comment by mikesamuel@gmail.com on 1 Aug 2008 at 6:21