From cb8f378115c7904f2731e5da2d1890d15a552973 Mon Sep 17 00:00:00 2001 From: teppeis Date: Wed, 5 Jun 2019 22:12:40 +0900 Subject: [PATCH 1/7] feat: prefer --depsJs and --namespace in detecting required namespaces --- lib/cli.js | 9 +++------ lib/default.js | 5 +++-- lib/parser.js | 14 +++++++------- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/lib/cli.js b/lib/cli.js index 91c52580..fa62ebaf 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -33,11 +33,8 @@ function setCommandOptions(command) { .option('-f, --fix-in-place', 'Fix the file in-place.') .option('--provideRoots ', 'Root namespaces to provide separated by comma.', list) .option('--requireRoots ', 'Root namespaces to require separated by comma.', list) - .option( - '--namespaceMethods ', - 'Methods or properties which are also namespaces separated by comma.', - list - ) + .option('--namespaceMethods ', 'DEPRECATED: Use --namespaces', list) + .option('--namespaces ', 'Provided namespaces separated by comma.', list) .option( '--replaceMap ', 'Methods or properties to namespaces mapping like "before1:after1,before2:after2".', @@ -149,7 +146,7 @@ async function main(argv, stdout, stderr, exit) { const options = { provideRoots: program.provideRoots, requireRoots: program.requireRoots, - namespaceMethods: symbols.concat(program.namespaceMethods), + providedNamespace: symbols.concat(program.namespaceMethods).concat(program.namespaces), replaceMap: program.replaceMap, }; const parser = new Parser(options); diff --git a/lib/default.js b/lib/default.js index d7469df2..aff72447 100644 --- a/lib/default.js +++ b/lib/default.js @@ -28,8 +28,9 @@ const ignorePackages = ['goog']; /** * @type {Array} + * @deprecated Use --depsJs */ -const namespaceMethods = [ +const providedNamespaces = [ 'goog.color.names', 'goog.date.month', 'goog.date.weekDay', @@ -66,7 +67,7 @@ exports.getReplaceMap = () => new Map(Object.entries(replaceMap)); /** * @return {Set} */ -exports.getNamespaceMethods = () => new Set(namespaceMethods); +exports.getProvidedNamespaces = () => new Set(providedNamespaces); /** * @return {Set} diff --git a/lib/parser.js b/lib/parser.js index 18b4092c..ebfb261c 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -38,10 +38,10 @@ const Parser = function(opt_options) { }); } - this.namespaceMethods_ = def.getNamespaceMethods(); - if (options.namespaceMethods) { - options.namespaceMethods.forEach(method => { - this.namespaceMethods_.add(method); + this.providedNamespaces_ = def.getProvidedNamespaces(); + if (options.providedNamespace) { + options.providedNamespace.forEach(method => { + this.providedNamespaces_.add(method); }); } @@ -517,7 +517,7 @@ Parser.prototype.getPackageName_ = function(name) { return prev; } }, []); - if (!this.isNamespaceMethod_(name)) { + if (!this.isProvidedNamespace_(name)) { lastname = names[names.length - 1]; if (/^[a-z$]/.test(lastname)) { // Remove the last method name. @@ -586,8 +586,8 @@ Parser.prototype.replaceMethod_ = function(method) { * @return {boolean} . * @private */ -Parser.prototype.isNamespaceMethod_ = function(method) { - return this.namespaceMethods_.has(method); +Parser.prototype.isProvidedNamespace_ = function(method) { + return this.providedNamespaces_.has(method); }; /** From 8fc66d9d629107597599d4d3541e779272c677ed Mon Sep 17 00:00:00 2001 From: teppeis Date: Wed, 5 Jun 2019 23:03:01 +0900 Subject: [PATCH 2/7] fix: remove --requireRoots BREAKING CHANGE: --requireRoots was removed, use --namespaces or --depsJs --- lib/cli.js | 2 -- lib/parser.js | 49 +++++++++++++++++++++++-------------------------- 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/lib/cli.js b/lib/cli.js index fa62ebaf..59ba74e9 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -32,7 +32,6 @@ function setCommandOptions(command) { .usage('[options] files...') .option('-f, --fix-in-place', 'Fix the file in-place.') .option('--provideRoots ', 'Root namespaces to provide separated by comma.', list) - .option('--requireRoots ', 'Root namespaces to require separated by comma.', list) .option('--namespaceMethods ', 'DEPRECATED: Use --namespaces', list) .option('--namespaces ', 'Provided namespaces separated by comma.', list) .option( @@ -145,7 +144,6 @@ async function main(argv, stdout, stderr, exit) { const src = await promisify(fs.readFile)(file, 'utf8'); const options = { provideRoots: program.provideRoots, - requireRoots: program.requireRoots, providedNamespace: symbols.concat(program.namespaceMethods).concat(program.namespaces), replaceMap: program.replaceMap, }; diff --git a/lib/parser.js b/lib/parser.js index ebfb261c..4421b40b 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -22,15 +22,6 @@ const Parser = function(opt_options) { this.provideRoots_ = def.getRoots(); } - if (options.requireRoots) { - this.requireRoots_ = new Set(options.requireRoots); - } else { - this.requireRoots_ = def.getRoots(); - this.provideRoots_.forEach(root => { - this.requireRoots_.add(root); - }); - } - this.replaceMap_ = def.getReplaceMap(); if (options.replaceMap) { options.replaceMap.forEach((value, key) => { @@ -173,7 +164,6 @@ Parser.prototype.extractToRequire_ = function(parsed, toProvide, comments, opt_r .map(this.toRequireMapper_.bind(this)) .concat(additional) .filter(this.isDefAndNotNull_) - .filter(this.requireRootFilter_.bind(this)) .sort() .reduce(this.uniq_, []); return difference(toRequire, toProvide); @@ -202,7 +192,7 @@ Parser.prototype.extractToRequireTypeFromJsDoc_ = function(comments) { }); return prev; }, []) - .filter(this.requireRootFilter_.bind(this)) + .filter(name => this.isProvidedNamespace_(name)) .sort() .reduce(this.uniq_, []); }; @@ -399,16 +389,6 @@ Parser.prototype.provideRootFilter_ = function(item) { return this.provideRoots_.has(root); }; -/** - * @param {string} item . - * @return {boolean} True if the item has a root namespace to extract. - * @private - */ -Parser.prototype.requireRootFilter_ = function(item) { - const root = item.split('.')[0]; - return this.requireRoots_.has(root); -}; - /** * @param {Object} use . * @return {?string} Used namespace. @@ -446,7 +426,7 @@ Parser.prototype.toProvideMapper_ = function(use) { */ Parser.prototype.toRequireMapper_ = function(use) { const name = use.name.join('.'); - return this.getPackageName_(name); + return this.getRequiredPackageName_(name); }; /** @@ -491,6 +471,23 @@ Parser.prototype.suppressFilter_ = function(comments, use) { return !suppressComment; }; +/** + * @param {string} name . + * @return {string|null} . + * @private + */ +Parser.prototype.getRequiredPackageName_ = function(name) { + let names = this.replaceMethod_(name).split('.'); + do { + const name = names.join('.'); + if (this.providedNamespaces_.has(name)) { + return name; + } + names = names.slice(0, -1); + } while (names.length > 0); + return null; +}; + /** * @param {string} name . * @return {?string} . @@ -582,12 +579,12 @@ Parser.prototype.replaceMethod_ = function(method) { }; /** - * @param {string} method Method name. - * @return {boolean} . + * @param {string} name + * @return {boolean} * @private */ -Parser.prototype.isProvidedNamespace_ = function(method) { - return this.providedNamespaces_.has(method); +Parser.prototype.isProvidedNamespace_ = function(name) { + return this.providedNamespaces_.has(name); }; /** From 3903ac4a27923b0a7cefb078f2b45e14ccc20bf6 Mon Sep 17 00:00:00 2001 From: teppeis Date: Wed, 5 Jun 2019 23:12:05 +0900 Subject: [PATCH 3/7] test: fix test --- test/fixtures/parse-es6-script/class.js | 8 ++++---- .../parse-es6-script/template-literal.js | 8 ++++---- test/fixtures/parse/@const.js | 4 ++-- test/fixtures/parse/@define.js | 4 ++-- test/fixtures/parse/@enum.js | 4 ++-- test/fixtures/parse/@extends-required.js | 6 +++--- test/fixtures/parse/@extends.js | 8 ++++---- test/fixtures/parse/@implements.js | 12 ++++++------ test/fixtures/parse/@param.js | 4 ++-- test/fixtures/parse/@return.js | 4 ++-- test/fixtures/parse/@this.js | 4 ++-- test/fixtures/parse/@type.js | 8 ++++---- test/fixtures/parse/@typedef.js | 6 ++++-- test/fixtures/parse/array.js | 4 ++-- test/fixtures/parse/assignment.js | 4 ++-- test/fixtures/parse/call.js | 4 ---- test/fixtures/parse/constructor.js | 19 ++++++------------- test/fixtures/parse/declare.js | 4 ++-- test/fixtures/parse/enum.js | 5 ++--- test/fixtures/parse/ignore_goog.js | 11 +---------- .../parse/inline_hint_suppress_require.js | 2 +- test/fixtures/parse/jsx.jsx | 18 +++++++++--------- test/fixtures/parse/member.js | 4 ++-- test/fixtures/parse/new.js | 6 +++--- test/fixtures/parse/post-replace.js | 4 ++-- .../fixtures/parse/private_static_property.js | 10 ++++------ test/fixtures/parse/prototype.js | 4 ++-- test/fixtures/parse/roots.js | 13 ------------- test/fixtures/parse/static_property.js | 11 ++++------- test/lib/asserts.js | 17 +++++++++++++++++ 30 files changed, 100 insertions(+), 120 deletions(-) delete mode 100644 test/fixtures/parse/roots.js diff --git a/test/fixtures/parse-es6-script/class.js b/test/fixtures/parse-es6-script/class.js index 88a5f567..0e039cd9 100644 --- a/test/fixtures/parse-es6-script/class.js +++ b/test/fixtures/parse-es6-script/class.js @@ -1,13 +1,13 @@ // ClassDeclaration -class C1 extends goog.Foo1 { +class C1 extends goog.Foo { constructor() { goog.foo.bar(); } } // ClassExpression -var C2 = class extends goog.Foo2 {}; +var C2 = class extends goog.Bar {}; -// toRequire: goog.Foo1 -// toRequire: goog.Foo2 +// toRequire: goog.Bar +// toRequire: goog.Foo // toRequire: goog.foo diff --git a/test/fixtures/parse-es6-script/template-literal.js b/test/fixtures/parse-es6-script/template-literal.js index 30c2dde3..fa55d006 100644 --- a/test/fixtures/parse-es6-script/template-literal.js +++ b/test/fixtures/parse-es6-script/template-literal.js @@ -1,6 +1,6 @@ -var a = `foo${goog.foo.bar}bar`; -var b = goog.tagged1.foo`foo${goog.tagged2.foo}bar`; +var a = `foo${goog.foo.aaa}bar`; +var b = goog.bar.aaa`foo${goog.baz.aaa}bar`; +// toRequire: goog.bar +// toRequire: goog.baz // toRequire: goog.foo -// toRequire: goog.tagged1 -// toRequire: goog.tagged2 diff --git a/test/fixtures/parse/@const.js b/test/fixtures/parse/@const.js index 5dfec8cf..2692c318 100644 --- a/test/fixtures/parse/@const.js +++ b/test/fixtures/parse/@const.js @@ -1,6 +1,6 @@ /** - * @const {goog.ui.Component} + * @const {goog.Foo} */ var foo; -// toRequireType: goog.ui.Component +// toRequireType: goog.Foo diff --git a/test/fixtures/parse/@define.js b/test/fixtures/parse/@define.js index 9e5e0623..87f3510c 100644 --- a/test/fixtures/parse/@define.js +++ b/test/fixtures/parse/@define.js @@ -1,4 +1,4 @@ -/** @define {goog.ui.Component} */ +/** @define {goog.Foo} */ var foo; -// toRequireType: goog.ui.Component +// toRequireType: goog.Foo diff --git a/test/fixtures/parse/@enum.js b/test/fixtures/parse/@enum.js index 204b2ee7..f81daf1e 100644 --- a/test/fixtures/parse/@enum.js +++ b/test/fixtures/parse/@enum.js @@ -1,6 +1,6 @@ /** - * @enum {goog.ui.Component} + * @enum {goog.Foo} */ var Foo = {}; -// toRequireType: goog.ui.Component +// toRequireType: goog.Foo diff --git a/test/fixtures/parse/@extends-required.js b/test/fixtures/parse/@extends-required.js index ce450696..df46ddca 100644 --- a/test/fixtures/parse/@extends-required.js +++ b/test/fixtures/parse/@extends-required.js @@ -1,9 +1,9 @@ /** * @constructor - * @extends {goog.Disposable} + * @extends {goog.Foo} */ var Bar = function() { }; -goog.inherits(Bar, goog.Disposable); +goog.inherits(Bar, goog.Foo); -// toRequire: goog.Disposable +// toRequire: goog.Foo diff --git a/test/fixtures/parse/@extends.js b/test/fixtures/parse/@extends.js index 93f2895a..614f5f6a 100644 --- a/test/fixtures/parse/@extends.js +++ b/test/fixtures/parse/@extends.js @@ -1,16 +1,16 @@ /** * @interface - * @extends {goog.disposable.IDisposable} + * @extends {goog.Foo} */ var Foo = function() { }; /** * @constructor - * @extends {goog.Disposable} + * @extends {goog.Bar} */ var Bar = function() { }; -// toRequireType: goog.Disposable -// toRequireType: goog.disposable.IDisposable +// toRequireType: goog.Bar +// toRequireType: goog.Foo diff --git a/test/fixtures/parse/@implements.js b/test/fixtures/parse/@implements.js index dcc593ff..9aed4c28 100644 --- a/test/fixtures/parse/@implements.js +++ b/test/fixtures/parse/@implements.js @@ -1,18 +1,18 @@ /** * @constructor - * @implements {goog.disposable.IDisposable} + * @implements {goog.Foo} */ var Foo = function() { }; /** - * @implements {goog.fx.anim.Animated} - * @implements {goog.fx.Transition} + * @implements {goog.Bar} + * @implements {goog.Baz} * @constructor */ var Bar = function() { }; -// toRequireType: goog.disposable.IDisposable -// toRequireType: goog.fx.Transition -// toRequireType: goog.fx.anim.Animated +// toRequireType: goog.Bar +// toRequireType: goog.Baz +// toRequireType: goog.Foo diff --git a/test/fixtures/parse/@param.js b/test/fixtures/parse/@param.js index 97d413b0..0f635f5a 100644 --- a/test/fixtures/parse/@param.js +++ b/test/fixtures/parse/@param.js @@ -1,7 +1,7 @@ /** - * @param {goog.ui.Component} component + * @param {goog.Foo} component */ function foo(component) { }; -// toRequireType: goog.ui.Component +// toRequireType: goog.Foo diff --git a/test/fixtures/parse/@return.js b/test/fixtures/parse/@return.js index 59743c59..af33afb8 100644 --- a/test/fixtures/parse/@return.js +++ b/test/fixtures/parse/@return.js @@ -1,7 +1,7 @@ /** - * @return {goog.ui.Component} + * @return {goog.Foo} */ var foo = function() { }; -// toRequireType: goog.ui.Component +// toRequireType: goog.Foo diff --git a/test/fixtures/parse/@this.js b/test/fixtures/parse/@this.js index f823913a..b5a0a09f 100644 --- a/test/fixtures/parse/@this.js +++ b/test/fixtures/parse/@this.js @@ -1,7 +1,7 @@ /** - * @this {goog.ui.Component} + * @this {goog.Foo} */ var foo = function() { }; -// toRequireType: goog.ui.Component +// toRequireType: goog.Foo diff --git a/test/fixtures/parse/@type.js b/test/fixtures/parse/@type.js index 3bc9b9a9..f8f2d678 100644 --- a/test/fixtures/parse/@type.js +++ b/test/fixtures/parse/@type.js @@ -1,13 +1,13 @@ /** - * @type {goog.ui.Component} + * @type {goog.Foo} */ var foo; /** * Uniqued * - * @type {goog.ui.Component} + * @type {goog.Foo} */ -var bar; +var foo2; -// toRequireType: goog.ui.Component +// toRequireType: goog.Foo diff --git a/test/fixtures/parse/@typedef.js b/test/fixtures/parse/@typedef.js index ec20c05e..0ce42dab 100644 --- a/test/fixtures/parse/@typedef.js +++ b/test/fixtures/parse/@typedef.js @@ -1,10 +1,10 @@ /** - * @typedef {Element} + * @typedef {goog.Foo} */ goog.foo.BarType; /** - * @typedef {Element} + * @typedef {goog.Bar} * @private */ goog.foo.PrivateType; @@ -12,3 +12,5 @@ goog.foo.PrivateType; goog.foo.NoJsDoc; // toProvide: goog.foo.BarType +// toRequireType: goog.Bar +// toRequireType: goog.Foo diff --git a/test/fixtures/parse/array.js b/test/fixtures/parse/array.js index 5b24379e..e8429ec5 100644 --- a/test/fixtures/parse/array.js +++ b/test/fixtures/parse/array.js @@ -1,3 +1,3 @@ -var foo = [goog.events.EventType.CLICK, goog.events.EventType.FOCUS]; +var foo = [goog.Foo.CLICK, goog.Foo.FOCUS]; -// toRequire: goog.events.EventType +// toRequire: goog.Foo diff --git a/test/fixtures/parse/assignment.js b/test/fixtures/parse/assignment.js index e1a4b851..0423de0f 100644 --- a/test/fixtures/parse/assignment.js +++ b/test/fixtures/parse/assignment.js @@ -1,4 +1,4 @@ -goog.foo.baz = goog.events.EventType.FOCUS; +goog.foo.focus = goog.Foo.FOCUS; // toProvide: goog.foo -// toRequire: goog.events.EventType +// toRequire: goog.Foo diff --git a/test/fixtures/parse/call.js b/test/fixtures/parse/call.js index aa1e5a11..158dc015 100644 --- a/test/fixtures/parse/call.js +++ b/test/fixtures/parse/call.js @@ -1,12 +1,8 @@ // start with a-z goog.foo.foo1(); -goog.foo.foo2(); goog.bar.bar1(); -goog.bar.bar2(); // start with $ goog.baz.$(); -goog.baz.$$(); -goog.baz.$F(); // only goog namespace should be require by default. foo.bar.bar2(); diff --git a/test/fixtures/parse/constructor.js b/test/fixtures/parse/constructor.js index fb51d62e..04030eed 100644 --- a/test/fixtures/parse/constructor.js +++ b/test/fixtures/parse/constructor.js @@ -1,29 +1,22 @@ /** * @constructor - * @extends {goog.ui.Component} + * @extends {goog.Foo} */ -goog.ui.Bubble = function() { +goog.aaa.Foo = function() { goog.base(this); - /** - * @type {goog.ui.Popup} - * @private - */ - this.popup_ = new goog.ui.Popup(); - /** * @type {string} * @private */ this.closeButtonId_ = this.makeId('cb'); }; -goog.inherits(goog.ui.Bubble, goog.ui.Component); +goog.inherits(goog.aaa.Foo, goog.Foo); /** */ -goog.ui.Bubble.prototype.hello = function() { +goog.aaa.Foo.prototype.hello = function() { }; -// toProvide: goog.ui.Bubble -// toRequire: goog.ui.Component -// toRequire: goog.ui.Popup +// toProvide: goog.aaa.Foo +// toRequire: goog.Foo diff --git a/test/fixtures/parse/declare.js b/test/fixtures/parse/declare.js index 0e9f753e..117cead8 100644 --- a/test/fixtures/parse/declare.js +++ b/test/fixtures/parse/declare.js @@ -1,3 +1,3 @@ -var foo = goog.events.EventType.CLICK; +var foo = goog.Foo.CLICK; -// toRequire: goog.events.EventType +// toRequire: goog.Foo diff --git a/test/fixtures/parse/enum.js b/test/fixtures/parse/enum.js index 096634d9..dd1effd0 100644 --- a/test/fixtures/parse/enum.js +++ b/test/fixtures/parse/enum.js @@ -1,4 +1,3 @@ -goog.events.listen(target, goog.events.EventType.CLICK, handler); +listen(target, goog.Foo.CLICK, handler); -// toRequire: goog.events -// toRequire: goog.events.EventType +// toRequire: goog.Foo diff --git a/test/fixtures/parse/ignore_goog.js b/test/fixtures/parse/ignore_goog.js index a3decbfa..e09856c0 100644 --- a/test/fixtures/parse/ignore_goog.js +++ b/test/fixtures/parse/ignore_goog.js @@ -1,10 +1 @@ -goog.foo(); -proto2.foo(); -soy.bar(); -soydata.bar(); -svgpan.bar(); - -// toRequire: proto2 -// toRequire: soy -// toRequire: soydata -// toRequire: svgpan +goog.aaa(); diff --git a/test/fixtures/parse/inline_hint_suppress_require.js b/test/fixtures/parse/inline_hint_suppress_require.js index c740112e..f54c0b88 100644 --- a/test/fixtures/parse/inline_hint_suppress_require.js +++ b/test/fixtures/parse/inline_hint_suppress_require.js @@ -3,7 +3,7 @@ goog.foo.foo(); goog.bar.bar(); goog.baz.baz(); // fixclosure: suppressRequire with comment -goog.bao.bao(); +goog.qux.qux(); // toRequire: goog.baz // toRequire: goog.foo diff --git a/test/fixtures/parse/jsx.jsx b/test/fixtures/parse/jsx.jsx index 6a62964e..bdf492d0 100644 --- a/test/fixtures/parse/jsx.jsx +++ b/test/fixtures/parse/jsx.jsx @@ -1,10 +1,10 @@ - - - {goog.foo.Content} - + + + {goog.Foo5} + -// toRequire: goog.foo.Attribute -// toRequire: goog.foo.Content -// toRequire: goog.foo.Element -// toRequire: goog.foo.SelfClosing -// toRequire: goog.foo.Spread +// toRequire: goog.Foo1 +// toRequire: goog.Foo2 +// toRequire: goog.Foo3 +// toRequire: goog.Foo4 +// toRequire: goog.Foo5 diff --git a/test/fixtures/parse/member.js b/test/fixtures/parse/member.js index d3fc9117..703bd13c 100644 --- a/test/fixtures/parse/member.js +++ b/test/fixtures/parse/member.js @@ -1,3 +1,3 @@ -var scheme = parts[goog.uri.utils.ComponentIndex.SCHEME]; +var scheme = parts[goog.Foo.SCHEME]; -// toRequire: goog.uri.utils.ComponentIndex +// toRequire: goog.Foo diff --git a/test/fixtures/parse/new.js b/test/fixtures/parse/new.js index b1f68c9b..356589d2 100644 --- a/test/fixtures/parse/new.js +++ b/test/fixtures/parse/new.js @@ -1,4 +1,4 @@ -new goog.positioning.AnchoredPosition(goog.positioning.Corner.BOTTOM_START); +new goog.Foo(goog.Bar.BOTTOM_START); -// toRequire: goog.positioning.AnchoredPosition -// toRequire: goog.positioning.Corner +// toRequire: goog.Bar +// toRequire: goog.Foo diff --git a/test/fixtures/parse/post-replace.js b/test/fixtures/parse/post-replace.js index 60457ba1..6988c977 100644 --- a/test/fixtures/parse/post-replace.js +++ b/test/fixtures/parse/post-replace.js @@ -1,3 +1,3 @@ -var NONE = goog.ui.KeyboardShortcutHandler.Modifiers.NONE; +var NONE = goog.Foo.Modifiers.NONE; -// toRequire: goog.ui.KeyboardShortcutHandler +// toRequire: goog.Foo diff --git a/test/fixtures/parse/private_static_property.js b/test/fixtures/parse/private_static_property.js index 85e302fc..c64d0b34 100644 --- a/test/fixtures/parse/private_static_property.js +++ b/test/fixtures/parse/private_static_property.js @@ -36,15 +36,13 @@ goog.p3.privateProp_ = goog.debug.Logger.getLogger('aaa'); * Private static method should not be require or provide. * @private */ -goog.p3.privateMethod_ = function() { +goog.p4.privateMethod_ = function() { }; /** + * To be provided */ -goog.p4.hello = function() { - goog.p3.privateProp_.info('bbb'); - goog.p3.privateMethod_(); +goog.p5.hello = function() { }; -// toRequire: goog.debug.Logger -// toProvide: goog.p4 +// toProvide: goog.p5 diff --git a/test/fixtures/parse/prototype.js b/test/fixtures/parse/prototype.js index d9921697..d9805270 100644 --- a/test/fixtures/parse/prototype.js +++ b/test/fixtures/parse/prototype.js @@ -1,3 +1,3 @@ -goog.date.Date.prototype.add.call(this, interval) +goog.Foo.prototype.add.call(this, interval) -// toRequire: goog.date.Date +// toRequire: goog.Foo diff --git a/test/fixtures/parse/roots.js b/test/fixtures/parse/roots.js deleted file mode 100644 index 41601c87..00000000 --- a/test/fixtures/parse/roots.js +++ /dev/null @@ -1,13 +0,0 @@ -goog.foo.bar(); -proto2.foo.bar(); -soy.foo.bar(); -soydata.foo.bar(); -svgpan.foo.bar(); -// baz is not a default root -baz.foo.bar(); - -// toRequire: goog.foo -// toRequire: proto2.foo -// toRequire: soy.foo -// toRequire: soydata.foo -// toRequire: svgpan.foo diff --git a/test/fixtures/parse/static_property.js b/test/fixtures/parse/static_property.js index 00e3b805..ef19403c 100644 --- a/test/fixtures/parse/static_property.js +++ b/test/fixtures/parse/static_property.js @@ -1,9 +1,6 @@ -/** - * @type {number} - */ -goog.debug.Console1.instance = 1; +goog.Bar.instance = 1; -goog.debug.Console2.instance.setCapturing(true); +goog.Baz.instance.setCapturing(true); -// toProvide: goog.debug.Console1 -// toRequire: goog.debug.Console2 +// toProvide: goog.Bar +// toRequire: goog.Baz diff --git a/test/lib/asserts.js b/test/lib/asserts.js index aa7ab785..05f9b371 100644 --- a/test/lib/asserts.js +++ b/test/lib/asserts.js @@ -62,6 +62,23 @@ exports.assertFile = (file, options) => { ignoredRequire.push(matches[1]); } + options = { + providedNamespace: [ + 'goog.foo', + 'goog.bar', + 'goog.baz', + 'goog.qux', + 'goog.Foo', + 'goog.Bar', + 'goog.Baz', + 'goog.Foo1', + 'goog.Foo2', + 'goog.Foo3', + 'goog.Foo4', + 'goog.Foo5', + ], + ...options, + }; const parser = new Parser(options); const info = parser.parse(src); // info.should.have.keys ['provided', 'required', 'toProvide', 'toRequire'] From d917484ba9a9c2e9309b4f1fb31f12e3dbe7542f Mon Sep 17 00:00:00 2001 From: teppeis Date: Thu, 6 Jun 2019 00:17:59 +0900 Subject: [PATCH 4/7] fix: apply ignorePackage for toRequire --- lib/parser.js | 2 +- test/fixtures/parse/ignore_goog.js | 2 +- test/lib/asserts.js | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/parser.js b/lib/parser.js index 4421b40b..6550c94f 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -480,7 +480,7 @@ Parser.prototype.getRequiredPackageName_ = function(name) { let names = this.replaceMethod_(name).split('.'); do { const name = names.join('.'); - if (this.providedNamespaces_.has(name)) { + if (this.providedNamespaces_.has(name) && !this.isIgnorePackage_(name)) { return name; } names = names.slice(0, -1); diff --git a/test/fixtures/parse/ignore_goog.js b/test/fixtures/parse/ignore_goog.js index e09856c0..797e51dc 100644 --- a/test/fixtures/parse/ignore_goog.js +++ b/test/fixtures/parse/ignore_goog.js @@ -1 +1 @@ -goog.aaa(); +goog.isDef(); diff --git a/test/lib/asserts.js b/test/lib/asserts.js index 05f9b371..01396271 100644 --- a/test/lib/asserts.js +++ b/test/lib/asserts.js @@ -64,6 +64,8 @@ exports.assertFile = (file, options) => { options = { providedNamespace: [ + // 'goog' is included in deps.js of Closure Library + 'goog', 'goog.foo', 'goog.bar', 'goog.baz', From b21a9495ac40b3528ffece304b4a53f5de64d53a Mon Sep 17 00:00:00 2001 From: teppeis Date: Thu, 6 Jun 2019 08:40:35 +0900 Subject: [PATCH 5/7] fix: apply replaceMap for toRequire --- lib/parser.js | 4 +- lib/visitor.js | 4 +- test/.fixclosurerc | 3 +- test/cli.js | 88 +++++++++++------------- test/fixtures/cli/.fixclosurerc1 | 3 +- test/fixtures/cli/config.js | 6 +- test/fixtures/cli/provideRoots.js | 3 - test/fixtures/cli/provideRootsDefault.js | 4 -- test/fixtures/cli/replacemap.js | 6 +- 9 files changed, 50 insertions(+), 71 deletions(-) diff --git a/lib/parser.js b/lib/parser.js index 6550c94f..234c79c1 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -477,9 +477,9 @@ Parser.prototype.suppressFilter_ = function(comments, use) { * @private */ Parser.prototype.getRequiredPackageName_ = function(name) { - let names = this.replaceMethod_(name).split('.'); + let names = name.split('.'); do { - const name = names.join('.'); + const name = this.replaceMethod_(names.join('.')); if (this.providedNamespaces_.has(name) && !this.isIgnorePackage_(name)) { return name; } diff --git a/lib/visitor.js b/lib/visitor.js index 14f21adf..c2cfca0a 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -128,11 +128,11 @@ function registerIdentifier_(node, parents, path) { function createMemberObject_(namespace, node, parentKey) { return { name: namespace, - node: node, + node, key: parentKey, }; } module.exports = { - leave: leave, + leave, }; diff --git a/test/.fixclosurerc b/test/.fixclosurerc index e3d8556e..50ccc311 100644 --- a/test/.fixclosurerc +++ b/test/.fixclosurerc @@ -1,2 +1 @@ ---provideRoots=foo,bar ---namespaceMethods=goog.foo.namespacemethod1 +--provideRoots=foo diff --git a/test/cli.js b/test/cli.js index ccb2bb8c..6be65ebb 100644 --- a/test/cli.js +++ b/test/cli.js @@ -20,6 +20,26 @@ class MockStdOut { } } +const providedNamespaces = [ + 'goog', + 'goog.foo', + 'goog.bar', + 'goog.baz', + 'goog.require.dup', + 'goog.require.ignore', + 'goog.require.missing', + 'goog.require.unnecessary', + 'goog.requireType.dup', + 'goog.requireType.ignore', + 'goog.requireType.missing', + 'goog.requireType.unnecessary', + 'goog.forwardDeclare.dup', + 'goog.forwardDeclare.ignore', + 'goog.forwardDeclare.missing', + 'goog.forwardDeclare.unnecessary', +]; +const ns = `--namespaces=${providedNamespaces.join(',')}`; + describe('Command line', () => { let out = null; let err = null; @@ -32,7 +52,7 @@ describe('Command line', () => { }); it('suceed with file argument', async () => { - await cli(cmd.concat(['test/fixtures/cli/ok.js']), out, err, exit); + await cli(cmd.concat(['test/fixtures/cli/ok.js', ns]), out, err, exit); exit.called.should.be.false; }); @@ -46,13 +66,13 @@ describe('Command line', () => { }); it('exit with 1 if the result is NG', async () => { - await cli(cmd.concat(['test/fixtures/cli/ng.js']), out, err, exit); + await cli(cmd.concat(['test/fixtures/cli/ng.js', ns]), out, err, exit); exit.calledOnce.should.be.true; exit.firstCall.args.should.eql([1]); }); it('output all error types', async () => { - await cli(cmd.concat(['test/fixtures/cli/all-ng-types.js']), out, err, exit); + await cli(cmd.concat(['test/fixtures/cli/all-ng-types.js', ns]), out, err, exit); exit.calledOnce.should.be.true; exit.firstCall.args.should.eql([1]); const expected = fs.readFileSync('test/fixtures/cli/all-ng-types.js.error.txt', 'utf8'); @@ -61,7 +81,7 @@ describe('Command line', () => { it('output all error types with --useForwardDeclare', async () => { await cli( - cmd.concat(['test/fixtures/cli/all-ng-types.js', '--useForwardDeclare']), + cmd.concat(['test/fixtures/cli/all-ng-types.js', '--useForwardDeclare', ns]), out, err, exit @@ -117,6 +137,7 @@ describe('Command line', () => { cmd.concat([ 'test/fixtures/cli/package_method.js', '--namespaceMethods=goog.foo.namespacemethod1,goog.foo.namespacemethod2', + ns, ]), out, err, @@ -127,7 +148,7 @@ describe('Command line', () => { it('--depsJs', async () => { await cli( - cmd.concat(['test/fixtures/cli/depsjs.js', '--depsJs=test/fixtures/cli/deps.js']), + cmd.concat(['test/fixtures/cli/depsjs.js', '--depsJs=test/fixtures/cli/deps.js', ns]), out, err, exit @@ -137,12 +158,12 @@ describe('Command line', () => { describe('--provideRoots', () => { it('includes "goog,proto2,soy,soydata,svgpan" by default', async () => { - await cli(cmd.concat(['test/fixtures/cli/provideRootsDefault.js']), out, err, exit); + await cli(cmd.concat(['test/fixtures/cli/provideRootsDefault.js', ns]), out, err, exit); sinon.assert.notCalled(exit); }); it('does not include "foo,bar" by default', async () => { - await cli(cmd.concat(['test/fixtures/cli/provideRoots.js']), out, err, exit); + await cli(cmd.concat(['test/fixtures/cli/provideRoots.js', ns]), out, err, exit); sinon.assert.called(exit); }); @@ -158,44 +179,7 @@ describe('Command line', () => { it('does not include default roots if a value is specified', async () => { await cli( - cmd.concat(['test/fixtures/cli/provideRootsDefault.js', '--provideRoots=foo,bar']), - out, - err, - exit - ); - sinon.assert.called(exit); - }); - }); - - describe('--requireRoots', () => { - it('includes "goog,proto2,soy,soydata,svgpan" by default', async () => { - await cli(cmd.concat(['test/fixtures/cli/requireRootsDefault.js']), out, err, exit); - sinon.assert.notCalled(exit); - }); - - it('does not include "foo,bar"', async () => { - await cli( - cmd.concat(['test/fixtures/cli/requireRoots.js', '--requireRoots=foo,bar']), - out, - err, - exit - ); - sinon.assert.notCalled(exit); - }); - - it('includes specified roots', async () => { - await cli( - cmd.concat(['test/fixtures/cli/requireRoots.js', '--requireRoots=foo,bar']), - out, - err, - exit - ); - sinon.assert.notCalled(exit); - }); - - it('does not include default roots if specified', async () => { - await cli( - cmd.concat(['test/fixtures/cli/requireRootsDefault.js', '--requireRoots=foo,bar']), + cmd.concat(['test/fixtures/cli/provideRootsDefault.js', '--provideRoots=foo,bar', ns]), out, err, exit @@ -208,7 +192,8 @@ describe('Command line', () => { await cli( cmd.concat([ 'test/fixtures/cli/replacemap.js', - '--replaceMap=goog.foo.foo:goog.bar.bar,goog.baz.Baz:goog.baz.Baaz', + '--replaceMap=goog.foo1:goog.foo,goog.bar1:goog.bar', + ns, ]), out, err, @@ -218,7 +203,12 @@ describe('Command line', () => { }); it('without --showSuccess', async () => { - await cli(cmd.concat(['test/fixtures/cli/ok.js', 'test/fixtures/cli/ng.js']), out, err, exit); + await cli( + cmd.concat(['test/fixtures/cli/ok.js', 'test/fixtures/cli/ng.js', ns]), + out, + err, + exit + ); exit.calledOnce.should.be.true; exit.firstCall.args.should.eql([1]); const expectedErr = fs.readFileSync('test/fixtures/cli/ng.js.txt', 'utf8'); @@ -228,7 +218,7 @@ describe('Command line', () => { it('--showSuccess', async () => { await cli( - cmd.concat(['test/fixtures/cli/ok.js', 'test/fixtures/cli/ng.js', '--showSuccess']), + cmd.concat(['test/fixtures/cli/ok.js', 'test/fixtures/cli/ng.js', '--showSuccess', ns]), out, err, exit @@ -245,7 +235,7 @@ describe('Command line', () => { const testFixInPlace = async (filename, options = []) => { const tmppath = tempy.file(`test/tmp/${filename}`); fs.copyFileSync(`test/fixtures/cli/${filename}`, tmppath); - await cli(cmd.concat([tmppath, '--fix-in-place']).concat(options), out, err, exit); + await cli(cmd.concat([tmppath, '--fix-in-place', ns]).concat(options), out, err, exit); exit.calledOnce.should.be.false; const fixedSrc = fs.readFileSync(tmppath, 'utf8'); const expected = fs.readFileSync(`test/fixtures/cli/${filename}.fixed.txt`, 'utf8'); diff --git a/test/fixtures/cli/.fixclosurerc1 b/test/fixtures/cli/.fixclosurerc1 index e3d8556e..50ccc311 100644 --- a/test/fixtures/cli/.fixclosurerc1 +++ b/test/fixtures/cli/.fixclosurerc1 @@ -1,2 +1 @@ ---provideRoots=foo,bar ---namespaceMethods=goog.foo.namespacemethod1 +--provideRoots=foo diff --git a/test/fixtures/cli/config.js b/test/fixtures/cli/config.js index b69c1fed..8f805bb2 100644 --- a/test/fixtures/cli/config.js +++ b/test/fixtures/cli/config.js @@ -1,5 +1,3 @@ -goog.require('goog.foo.namespacemethod1'); -goog.require('foo.foo'); +goog.provide('foo.bar'); -goog.foo.namespacemethod1(hoge); -foo.foo.method(hoge); +foo.bar.baz = function() {}; diff --git a/test/fixtures/cli/provideRoots.js b/test/fixtures/cli/provideRoots.js index c08216de..1709e0a6 100644 --- a/test/fixtures/cli/provideRoots.js +++ b/test/fixtures/cli/provideRoots.js @@ -1,9 +1,6 @@ goog.provide('bar.Bar'); goog.provide('foo.Foo'); -goog.require('bar.bar'); -goog.require('foo.foo'); - foo.Foo = function() {}; bar.Bar = function() {}; diff --git a/test/fixtures/cli/provideRootsDefault.js b/test/fixtures/cli/provideRootsDefault.js index 7ca00fba..f9aaa621 100644 --- a/test/fixtures/cli/provideRootsDefault.js +++ b/test/fixtures/cli/provideRootsDefault.js @@ -5,10 +5,6 @@ goog.provide('soydata.Foo'); goog.provide('svgpan.Foo'); goog.require('goog.foo'); -goog.require('proto2.foo'); -goog.require('soy.foo'); -goog.require('soydata.foo'); -goog.require('svgpan.foo'); goog.Foo = function() {}; proto2.Foo = function() {}; diff --git a/test/fixtures/cli/replacemap.js b/test/fixtures/cli/replacemap.js index 4025da8d..3200b707 100644 --- a/test/fixtures/cli/replacemap.js +++ b/test/fixtures/cli/replacemap.js @@ -1,7 +1,7 @@ goog.require('goog.bar'); -goog.require('goog.baz.Baaz'); +goog.require('goog.foo'); goog.require('goog.dispose'); goog.disposeAll(foo, bar); -goog.foo.foo(foo, bar); -goog.baz.Baz(foo, bar); +goog.foo1.foo2.foo3(foo, bar); +goog.bar1.bar2.bar3(foo, bar); From 5c6f4a620ee79889a4076fa1b81d41f7ebf771eb Mon Sep 17 00:00:00 2001 From: teppeis Date: Thu, 6 Jun 2019 12:22:47 +0900 Subject: [PATCH 6/7] fix: provide parent namespaces of private props --- lib/parser.js | 45 +++++++++---------- .../fixtures/parse/private_static_property.js | 32 +++++++------ 2 files changed, 40 insertions(+), 37 deletions(-) diff --git a/lib/parser.js b/lib/parser.js index 234c79c1..fc6cada4 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -496,9 +496,6 @@ Parser.prototype.getRequiredPackageName_ = function(name) { Parser.prototype.getPackageName_ = function(name) { name = this.replaceMethod_(name); let names = name.split('.'); - if (this.isPrivateProp_(names)) { - return null; - } let lastname = names[names.length - 1]; // Remove calling with apply or call. if (lastname === 'apply' || lastname === 'call') { @@ -514,32 +511,34 @@ Parser.prototype.getPackageName_ = function(name) { return prev; } }, []); - if (!this.isProvidedNamespace_(name)) { + lastname = names[names.length - 1]; + if (/^[a-z$]/.test(lastname)) { + // Remove the last method name. + names.pop(); + } + + while (names.length > 0) { lastname = names[names.length - 1]; - if (/^[a-z$]/.test(lastname)) { - // Remove the last method name. + if (/^[A-Z][_0-9A-Z]+$/.test(lastname)) { + // Remove the last constant name. names.pop(); + } else { + break; } + } - while (names.length > 0) { - lastname = names[names.length - 1]; - if (/^[A-Z][_0-9A-Z]*$/.test(lastname)) { - // Remove the last constant name. - names.pop(); - } else { - break; - } + // Remove the static property. + const PARENT_CLASS_INDEX_FROM_LAST = 2; + if (names.length > PARENT_CLASS_INDEX_FROM_LAST) { + lastname = names[names.length - 1]; + const parentClass = names[names.length - PARENT_CLASS_INDEX_FROM_LAST]; + if (/^[a-z]/.test(lastname) && /^[A-Z]/.test(parentClass)) { + names.pop(); } + } - // Remove the static property. - const PARENT_CLASS_INDEX_FROM_LAST = 2; - if (names.length > PARENT_CLASS_INDEX_FROM_LAST) { - lastname = names[names.length - 1]; - const parentClass = names[names.length - PARENT_CLASS_INDEX_FROM_LAST]; - if (/^[a-z]/.test(lastname) && /^[A-Z]/.test(parentClass)) { - names.pop(); - } - } + if (this.isPrivateProp_(names)) { + return null; } const pkg = names.join('.'); diff --git a/test/fixtures/parse/private_static_property.js b/test/fixtures/parse/private_static_property.js index c64d0b34..7c044b7c 100644 --- a/test/fixtures/parse/private_static_property.js +++ b/test/fixtures/parse/private_static_property.js @@ -7,42 +7,46 @@ goog.p1.Private_ = function() { }; /** - * Nested private class. + * A nested private class should not be provided. * @constructor * @private */ -goog.p2.Private1_.Private2_.Private3_ = function() { +goog.p2.Private1_.Private2_ = function() { }; /** - * Method of nested private class. + * A nested private class should not be provided. + * @constructor + * @private */ -goog.p2.Private1_.Private2_.Private3_.prototype.hello = function() { +goog.p2.Private1_.Private2_.Private3_ = function() { }; /** - * Static method of nested private class. + * A method of nested private class should not be provided. */ -goog.p2.Private1_.Private2_.Private3_.publicStatic = function() { +goog.p2.Private1_.Private2_.Private3_.prototype.hello = function() { }; /** - * Private static property should not be require or provide. - * @private + * A static method of nested private class should not be provided. */ -goog.p3.privateProp_ = goog.debug.Logger.getLogger('aaa'); +goog.p2.Private1_.Private2_.Private3_.publicStatic = function() { +}; /** - * Private static method should not be require or provide. + * Private static method should not be provided, + * but the parent namespace should be provided. * @private */ -goog.p4.privateMethod_ = function() { +goog.p3.privateMethod_ = function() { }; /** - * To be provided + * Normal case. The parent namespace should be provided. */ -goog.p5.hello = function() { +goog.p4.hello = function() { }; -// toProvide: goog.p5 +// toProvide: goog.p3 +// toProvide: goog.p4 From d19e1e0e8dc30fd14e943cbe73ff5f1253eb68b2 Mon Sep 17 00:00:00 2001 From: teppeis Date: Thu, 6 Jun 2019 23:14:10 +0900 Subject: [PATCH 7/7] fix: provide namespaces from deps.js --- lib/parser.js | 45 ++++++++++++----------- test/fixtures/parse/provide-namespaces.js | 4 ++ 2 files changed, 28 insertions(+), 21 deletions(-) create mode 100644 test/fixtures/parse/provide-namespaces.js diff --git a/lib/parser.js b/lib/parser.js index fc6cada4..64f6c311 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -400,7 +400,7 @@ Parser.prototype.toProvideMapper_ = function(use) { switch (use.node.type) { case Syntax.AssignmentExpression: if (use.key === 'left' && use.node.loc.start.column === 0) { - return this.getPackageName_(name); + return this.getProvidedPackageName_(name); } break; case Syntax.ExpressionStatement: @@ -409,7 +409,7 @@ Parser.prototype.toProvideMapper_ = function(use) { } typeDefComments = this.getTypedefComments_(use.node.leadingComments); if (typeDefComments.length > 0) { - return this.getPackageName_(name); + return this.getProvidedPackageName_(name); } break; @@ -493,7 +493,7 @@ Parser.prototype.getRequiredPackageName_ = function(name) { * @return {?string} . * @private */ -Parser.prototype.getPackageName_ = function(name) { +Parser.prototype.getProvidedPackageName_ = function(name) { name = this.replaceMethod_(name); let names = name.split('.'); let lastname = names[names.length - 1]; @@ -511,29 +511,32 @@ Parser.prototype.getPackageName_ = function(name) { return prev; } }, []); - lastname = names[names.length - 1]; - if (/^[a-z$]/.test(lastname)) { - // Remove the last method name. - names.pop(); - } - while (names.length > 0) { + if (!this.isProvidedNamespace_(name)) { lastname = names[names.length - 1]; - if (/^[A-Z][_0-9A-Z]+$/.test(lastname)) { - // Remove the last constant name. + if (/^[a-z$]/.test(lastname)) { + // Remove the last method name. names.pop(); - } else { - break; } - } - // Remove the static property. - const PARENT_CLASS_INDEX_FROM_LAST = 2; - if (names.length > PARENT_CLASS_INDEX_FROM_LAST) { - lastname = names[names.length - 1]; - const parentClass = names[names.length - PARENT_CLASS_INDEX_FROM_LAST]; - if (/^[a-z]/.test(lastname) && /^[A-Z]/.test(parentClass)) { - names.pop(); + while (names.length > 0) { + lastname = names[names.length - 1]; + if (/^[A-Z][_0-9A-Z]+$/.test(lastname)) { + // Remove the last constant name. + names.pop(); + } else { + break; + } + } + + // Remove the static property. + const PARENT_CLASS_INDEX_FROM_LAST = 2; + if (names.length > PARENT_CLASS_INDEX_FROM_LAST) { + lastname = names[names.length - 1]; + const parentClass = names[names.length - PARENT_CLASS_INDEX_FROM_LAST]; + if (/^[a-z]/.test(lastname) && /^[A-Z]/.test(parentClass)) { + names.pop(); + } } } diff --git a/test/fixtures/parse/provide-namespaces.js b/test/fixtures/parse/provide-namespaces.js new file mode 100644 index 00000000..122864f1 --- /dev/null +++ b/test/fixtures/parse/provide-namespaces.js @@ -0,0 +1,4 @@ +goog.foo = function() {}; +goog.aaa = function() {}; + +// toProvide: goog.foo