From d1b1d2691ab8d260a4ce0d6ee47598972e6284aa Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 10 Dec 2011 11:06:34 -0500 Subject: [PATCH] Fixed initial warnings provided by Closure Compiler Getting ready for release means that we need to rest assured that everything is operating as it should. Tests do an excellent job at aiding in this, but they cannot cover everything. For example, a simple missing comma in a variable declaration list could have terrible, global consequences. --- Makefile | 7 +++++-- lib/ClassBuilder.js | 13 +++++-------- lib/VisibilityObjectFactory.js | 8 ++++---- lib/interface.js | 2 +- lib/util.js | 8 ++++---- test/MemberBuilder/VisibilityTest.js | 6 +++--- test/MethodWrappersTest.js | 6 +++--- test/VisibilityObjectFactoryTest.js | 2 +- test/inc-testcase.js | 11 ++++++----- test/test-class-constructor.js | 2 +- test/test-class-extend.js | 2 +- test/test-class-name.js | 4 ++-- test/test-class-parent.js | 2 +- test/test-class_builder-final.js | 2 +- test/test-class_builder-member-restrictions.js | 9 +++++---- test/test-class_builder-static.js | 8 ++++---- test/test-class_builder-visibility.js | 2 +- test/test-util-clone.js | 4 ++-- test/test-util-copy.js | 2 +- test/test-util-prop-parse-keywords.js | 2 +- test/test-util-prop-parse.js | 4 ++-- tools/externs-global.js | 9 +++++++++ 22 files changed, 63 insertions(+), 52 deletions(-) create mode 100644 tools/externs-global.js diff --git a/Makefile b/Makefile index 9b0da39..3415965 100644 --- a/Makefile +++ b/Makefile @@ -58,13 +58,16 @@ perf-%.js: default # minificatino process uses Google Closure compiler min: build/ease.min.js build/ease-full.min.js $(path_build)/browser-test-min.html \ - | combine + $(path_tools)/externs-global.js | combine $(compiler): wget -O- http://closure-compiler.googlecode.com/files/compiler-latest.tar.gz \ | tar -C $(path_tools) -xzv compiler.jar build/%.min.js: build/%.js $(compiler) cat $(path_tools)/license.tpl > $@ - java -jar $(compiler) --js $< >> $@ || rm $@ + java -jar $(compiler) \ + --warning_level VERBOSE \ + --externs $(path_tools)/externs-global.js \ + --js $< >> $@ || rm $@ install: doc-info [ -d $(path_info_install) ] || mkdir -p $(path_info_install) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index 73f545c..019a3a5 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -64,7 +64,7 @@ var util = require( __dirname + '/util' ), * is performed only for methods. This is for performance reasons. We do not * have a situation where we will want to check for properties as well. * - * @type {Object.} */ public_methods = { '__construct': true, @@ -296,7 +296,7 @@ exports.prototype.build = function extend() static_members = { methods: this._memberBuilder.initMembers(), props: this._memberBuilder.initMembers(), - } + }, abstract_methods = util.clone( exports.getMeta( base ).abstractMethods ) @@ -438,10 +438,7 @@ exports.prototype.buildMembers = function buildMembers( // disallow use of our internal __initProps() method if ( reserved_members[ name ] === true ) { - throw Error( - ( ( cname ) ? cname + '::' : '' ) + - ( name + " is reserved" ) - ); + throw Error( name + " is reserved" ); } // if a member was defined multiple times in the same class @@ -730,7 +727,7 @@ exports.prototype.createAbstractCtor = function( cname ) * * @param {{public: Object, protected: Object, private: Object}} members * - * @param {function() ctor class + * @param {function()} ctor class * @param {number} cid class id * * @return {undefined} @@ -1095,7 +1092,7 @@ function attachInstanceOf( instance ) * @param {function()} inst instance that the method is being called from * @param {number} cid class id * - * @return {Object,null} instance object if found, otherwise null + * @return {Object|null} instance object if found, otherwise null */ exports.getMethodInstance = function( inst, cid ) { diff --git a/lib/VisibilityObjectFactory.js b/lib/VisibilityObjectFactory.js index d3c7510..1be7f8a 100644 --- a/lib/VisibilityObjectFactory.js +++ b/lib/VisibilityObjectFactory.js @@ -109,7 +109,7 @@ exports.prototype._createPrivateLayer = function( atop_of, properties ) obj_ctor.prototype = atop_of; // we'll be returning an instance, so that the prototype takes effect - obj = new obj_ctor(); + var obj = new obj_ctor(); // All protected properties need to be proxied from the private object // (which will be passed as the context) to the object containing protected @@ -142,7 +142,7 @@ exports.prototype._doSetup = function( // copy over the methods if ( methods !== undefined ) { - for ( method_name in methods ) + for ( var method_name in methods ) { if ( hasOwn.call( methods, method_name ) ) { @@ -172,7 +172,7 @@ exports.prototype._doSetup = function( } // initialize private/protected properties and store in instance data - for ( prop in properties ) + for ( var prop in properties ) { if ( hasOwn.call( properties, prop ) ) { @@ -205,7 +205,7 @@ exports.prototype.createPropProxy = function( base, dest, props ) { var hasOwn = Object.prototype.hasOwnProperty; - for ( prop in props ) + for ( var prop in props ) { if ( !( hasOwn.call( props, prop ) ) ) { diff --git a/lib/interface.js b/lib/interface.js index 69b74e6..6c9149a 100644 --- a/lib/interface.js +++ b/lib/interface.js @@ -51,7 +51,7 @@ var util = require( __dirname + '/util' ), */ module.exports = function() { - var type = typeof arguments[ 0 ] + var type = typeof arguments[ 0 ], result = null ; diff --git a/lib/util.js b/lib/util.js index 281f269..90b832f 100644 --- a/lib/util.js +++ b/lib/util.js @@ -153,7 +153,7 @@ exports.clone = function clone( data, deep ) hasOwn = Object.prototype.hasOwnProperty; // copy data to the new object - for ( prop in data ) + for ( var prop in data ) { if ( hasOwn.call( data, prop ) ) { @@ -204,7 +204,7 @@ exports.copyTo = function( dest, src, deep ) // slower; supports getters/setters if ( can_define_prop ) { - for ( prop in src ) + for ( var prop in src ) { data = Object.getOwnPropertyDescriptor( src, prop ); @@ -228,7 +228,7 @@ exports.copyTo = function( dest, src, deep ) // quick (keep if statement out of the loop) else { - for ( prop in src ) + for ( var prop in src ) { // normal copy; cloned if deep, otherwise by reference dest[ prop ] = ( deep ) @@ -274,7 +274,7 @@ exports.propParse = function( data, options ) // for each of the given properties, determine what type of property we're // dealing with (in the classic OO sense) - for ( prop in data ) + for ( var prop in data ) { // ignore properties of instance prototypes if ( !( hasOwn.call( data, prop ) ) ) diff --git a/test/MemberBuilder/VisibilityTest.js b/test/MemberBuilder/VisibilityTest.js index f3398af..bb6575b 100644 --- a/test/MemberBuilder/VisibilityTest.js +++ b/test/MemberBuilder/VisibilityTest.js @@ -81,7 +81,7 @@ require( 'common' ).testCase( _self.incAssertCount(); - for ( level in _self.members ) + for ( var level in _self.members ) { if ( typeof _self.members[ level ][ name ] === 'undefined' ) { @@ -264,7 +264,7 @@ require( 'common' ).testCase( ; // ensure we can initialize the values of each visibility level - for ( vis in test ) + for ( var vis in test ) { this.assertStrictEqual( test[ vis ], members[ vis ], "Visibility level '" + vis + "' cannot be initialized" @@ -282,7 +282,7 @@ require( 'common' ).testCase( var _self = this, tests = [ 'public', 'protected', 'private' ]; - for ( i in tests ) + for ( var i in tests ) { _self.basicVisPropTest( tests[ i ] ); _self.basicVisMethodTest( tests[ i ] ); diff --git a/test/MethodWrappersTest.js b/test/MethodWrappersTest.js index 1b1cb6b..5474981 100644 --- a/test/MethodWrappersTest.js +++ b/test/MethodWrappersTest.js @@ -23,7 +23,7 @@ var common = require( './common' ), assert = require( 'assert' ), - util = common.require( 'util' ) + util = common.require( 'util' ), sut = common.require( 'MethodWrappers' ) ; @@ -45,7 +45,7 @@ var common = require( './common' ), { called = true; return instance; - } + }, method = sut.standard.wrapNew( function() @@ -152,7 +152,7 @@ var common = require( './common' ), super_called = true; }, null, 0, getInst - ) + ), // the overriding method override = sut.standard.wrapOverride( diff --git a/test/VisibilityObjectFactoryTest.js b/test/VisibilityObjectFactoryTest.js index d4a1da8..e726707 100644 --- a/test/VisibilityObjectFactoryTest.js +++ b/test/VisibilityObjectFactoryTest.js @@ -111,7 +111,7 @@ var VisibilityObjectFactory = common.require( 'VisibilityObjectFactory' ), sut.createPropProxy( base, dest, props ); // check to ensure the properties are properly proxied - for ( prop in props ) + for ( var prop in props ) { dest[ prop ] = val; diff --git a/test/inc-testcase.js b/test/inc-testcase.js index 4390828..a68f447 100644 --- a/test/inc-testcase.js +++ b/test/inc-testcase.js @@ -25,6 +25,7 @@ var assert = require( 'assert' ), assert_wrapped = {}, acount = 0, icount = 0, + scount = 0, skpcount = 0, tcount = 0, @@ -41,7 +42,7 @@ var assert = require( 'assert' ), // wrap each of the assertions so that we can keep track of the number of times // that they were invoked -for ( f in assert ) +for ( var f in assert ) { var _assert_cur = assert[ f ]; @@ -105,7 +106,7 @@ module.exports = function( test_case ) delete test_case.setUp; // run each test in the case - for ( test in test_case ) + for ( var test in test_case ) { // xUnit-style setup if ( setUp ) @@ -282,7 +283,7 @@ function getMock( proto ) proto = Mock.prototype = new P() ; - for ( i in proto ) + for ( var i in proto ) { // only mock out methods if ( typeof proto[ i ] !== 'function' ) @@ -350,7 +351,7 @@ function outputTestFailures( failures ) var err = '', i = failures.length; - for ( i in failures ) + for ( var i in failures ) { var failure = failures[ i ]; @@ -368,7 +369,7 @@ function outputTestFailures( failures ) throw Error( err ); } - for ( i = 0; i < failures.length; i++ ) + for ( var i = 0; i < failures.length; i++ ) { cur = failures[ i ]; diff --git a/test/test-class-constructor.js b/test/test-class-constructor.js index e036dab..ebf14d9 100644 --- a/test/test-class-constructor.js +++ b/test/test-class-constructor.js @@ -29,7 +29,7 @@ var common = require( './common' ), // will still be set even if the context of the constructor is wrong var construct_count = 0, construct_context = null, - construct_args = null + construct_args = null, // create a basic test class Foo = Class.extend( diff --git a/test/test-class-extend.js b/test/test-class-extend.js index 99c4246..50e9890 100644 --- a/test/test-class-extend.js +++ b/test/test-class-extend.js @@ -36,7 +36,7 @@ var foo_props = { Class( foo_props ), ], - class_count = classes.length + class_count = classes.length, // will hold the class being tested Foo = null diff --git a/test/test-class-name.js b/test/test-class-name.js index 57bfd24..3746cc3 100644 --- a/test/test-class-name.js +++ b/test/test-class-name.js @@ -199,8 +199,8 @@ var common = require( './common' ), ( function testCanCreateNamedClassUsingStagingMethod() { var name = 'Foo', - named = Class( name ).extend( {} ) - namedi = Class( name ).implement( Interface( {} ) ).extend( {} ) + named = Class( name ).extend( {} ), + namedi = Class( name ).implement( Interface( {} ) ).extend( {} ), // we should also be able to extend classes in this manner namede = Class( name ).implement( Interface( {} ) ).extend( named, {} ) diff --git a/test/test-class-parent.js b/test/test-class-parent.js index 9b3d156..0d7af88 100644 --- a/test/test-class-parent.js +++ b/test/test-class-parent.js @@ -23,7 +23,7 @@ var common = require( './common' ), assert = require( 'assert' ), - Class = common.require( 'class' ); + Class = common.require( 'class' ), // we store these outside of the class to ensure that visibility bugs do not // get in the way of our assertions diff --git a/test/test-class_builder-final.js b/test/test-class_builder-final.js index 2e8c91e..4002ce7 100644 --- a/test/test-class_builder-final.js +++ b/test/test-class_builder-final.js @@ -28,7 +28,7 @@ var common = require( './common' ), common.require( 'VisibilityObjectFactoryFactory' ).fromEnvironment() ), - Class = common.require( 'class' ) + Class = common.require( 'class' ), FinalClass = common.require( 'class_final' ) ; diff --git a/test/test-class_builder-member-restrictions.js b/test/test-class_builder-member-restrictions.js index 54fb005..a852ef5 100644 --- a/test/test-class_builder-member-restrictions.js +++ b/test/test-class_builder-member-restrictions.js @@ -24,6 +24,7 @@ var common = require( './common' ), assert = require( 'assert' ), + Class = common.require( 'class' ), ClassBuilder = common.require( 'ClassBuilder' ), builder = ClassBuilder( common.require( 'MemberBuilder' )(), @@ -84,7 +85,7 @@ var common = require( './common' ), count = 0; // test each of the reserved members - for ( name in reserved ) + for ( var name in reserved ) { // properties assert['throws']( @@ -137,7 +138,7 @@ var common = require( './common' ), "Can retrieve hash of forced-public methods" ); - for ( name in pub ) + for ( var name in pub ) { count++; } @@ -176,12 +177,12 @@ var common = require( './common' ), var pub = ClassBuilder.getForcedPublicMethods(); // test each of the reserved members - for ( name in pub ) + for ( var name in pub ) { assert['throws']( function() { var obj = {}; - obj[ name ] = function() {}; + obj[ 'private ' + name ] = function() {}; Class( obj ); }, Error, "Forced-public methods must be declared as public" ); diff --git a/test/test-class_builder-static.js b/test/test-class_builder-static.js index c6b5a2e..305ec68 100644 --- a/test/test-class_builder-static.js +++ b/test/test-class_builder-static.js @@ -22,7 +22,7 @@ */ var common = require( './common' ), - fallback = common.require( 'util' ).definePropertyFallback() + fallback = common.require( 'util' ).definePropertyFallback(), // XXX: get rid of this disgusting mess; we're mid-refactor and all these // dependencies should not be necessary for testing @@ -183,7 +183,7 @@ require( 'common' ).testCase( // we must define in this manner so older engines won't blow up due to // syntax errors var def = {}, - val = 'baz' + val = 'baz', called = []; Object.defineProperty( def, 'public static foo', { @@ -615,7 +615,7 @@ require( 'common' ).testCase( this.foo = val; }, }, - val = 'baz' + val = 'baz', called = []; Object.defineProperty( def, 'protected static foo', { @@ -915,7 +915,7 @@ require( 'common' ).testCase( this.foo = val; }, }, - val = 'baz' + val = 'baz', called = []; Object.defineProperty( def, 'private static foo', { diff --git a/test/test-class_builder-visibility.js b/test/test-class_builder-visibility.js index 7fd6230..84214a1 100644 --- a/test/test-class_builder-visibility.js +++ b/test/test-class_builder-visibility.js @@ -62,7 +62,7 @@ require( 'common' ).testCase( */ 'Self property references instance rather than property object': function() { - var result = null + var result = null, ref = null, foo = this.builder.build( { diff --git a/test/test-util-clone.js b/test/test-util-clone.js index 433660a..cf8b649 100644 --- a/test/test-util-clone.js +++ b/test/test-util-clone.js @@ -52,7 +52,7 @@ for ( var i = 0, len = arr.length; i < len; i++ ) } // ensure object was properly cloned -for ( prop in obj ) +for ( var prop in obj ) { assert.equal( obj2[ prop ], @@ -94,7 +94,7 @@ while ( deep_i-- ) ); } -for ( prop in deep_obj ) +for ( var prop in deep_obj ) { assert.ok( ( deep_obj2[ prop ] !== deep_obj[ prop ] ), diff --git a/test/test-util-copy.js b/test/test-util-copy.js index 405d04a..96be034 100644 --- a/test/test-util-copy.js +++ b/test/test-util-copy.js @@ -49,7 +49,7 @@ var common = require( './common' ), copyTo( dest, src ); - for ( key in src ) + for ( var key in src ) { assert.deepEqual( src[ key ], dest[ key ], "Copies by reference by default" diff --git a/test/test-util-prop-parse-keywords.js b/test/test-util-prop-parse-keywords.js index 45ff875..cacaf6e 100644 --- a/test/test-util-prop-parse-keywords.js +++ b/test/test-util-prop-parse-keywords.js @@ -161,7 +161,7 @@ var common = require( './common' ), }, } ); - for ( prop in parsed_keywords ) + for ( var prop in parsed_keywords ) { assert.deepEqual( parsed_keywords[ prop ], diff --git a/test/test-util-prop-parse.js b/test/test-util-prop-parse.js index a3051e0..dc9cc02 100644 --- a/test/test-util-prop-parse.js +++ b/test/test-util-prop-parse.js @@ -61,7 +61,7 @@ if ( get_set ) var chk_each = {}; -for ( i in data ) +for ( var i in data ) { chk_each[ i ] = 1; } @@ -152,7 +152,7 @@ if ( get_set ) var chk_each_count = 0; -for ( item in chk_each ) +for ( var item in chk_each ) { chk_each_count++; } diff --git a/tools/externs-global.js b/tools/externs-global.js new file mode 100644 index 0000000..bc0bea8 --- /dev/null +++ b/tools/externs-global.js @@ -0,0 +1,9 @@ +/** + * Common global externs for Closure Compiler + */ + +// available in most modern environments or extensions +var console = {}; + +// Node.js +var process = {};