From 733f0a7af26c11752f19011b382e1075faf8a1e6 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Tue, 21 Jul 2015 15:54:13 -0400 Subject: [PATCH] Enforce usage of populated options for deprecate and warn. --- packages/ember-debug/lib/deprecate.js | 36 ++++++- packages/ember-debug/lib/warn.js | 24 ++++- packages/ember-debug/tests/main_test.js | 123 ++++++++++++++++++++---- 3 files changed, 159 insertions(+), 24 deletions(-) diff --git a/packages/ember-debug/lib/deprecate.js b/packages/ember-debug/lib/deprecate.js index fd72e52850f..29e122d8587 100644 --- a/packages/ember-debug/lib/deprecate.js +++ b/packages/ember-debug/lib/deprecate.js @@ -71,6 +71,12 @@ registerHandler(function raiseOnDeprecation(message, options, next) { } }); +export let missingOptionsDeprecation = 'When calling `Ember.deprecate` you ' + + 'must provide an `options` hash as the third parameter. ' + + '`options` should include `id` and `until` properties.'; +export let missingOptionsIdDeprecation = 'When calling `Ember.deprecate` you must provide `id` in options.'; +export let missingOptionsUntilDeprecation = 'When calling `Ember.deprecate` you must provide `until` in options.'; + /** Display a deprecation warning with the provided message and a stack trace (Chrome and Firefox only). Ember build tools will remove any calls to @@ -78,16 +84,40 @@ registerHandler(function raiseOnDeprecation(message, options, next) { @method deprecate @param {String} message A description of the deprecation. - @param {Boolean|Function} test An optional boolean. If falsy, the deprecation + @param {Boolean|Function} test A boolean. If falsy, the deprecation will be displayed. If this is a function, it will be executed and its return value will be used as condition. - @param {Object} options An optional object that can be used to pass + @param {Object} options An object that can be used to pass in a `url` to the transition guide on the emberjs.com website, and a unique `id` for this deprecation. The `id` can be used by Ember debugging tools to change the behavior (raise, log or silence) for that specific deprecation. The `id` should be namespaced by dots, e.g. "view.helper.select". @public */ -export default function() { +export default function deprecate(message, test, options) { + if (!options || (!options.id && !options.until)) { + deprecate( + missingOptionsDeprecation, + false, + { id: 'ember-debug.deprecate-options-missing', until: '3.0.0' } + ); + } + + if (options && !options.id) { + deprecate( + missingOptionsIdDeprecation, + false, + { id: 'ember-debug.deprecate-id-missing', until: '3.0.0' } + ); + } + + if (options && !options.until) { + deprecate( + missingOptionsUntilDeprecation, + options && options.until, + { id: 'ember-debug.deprecate-until-missing', until: '3.0.0' } + ); + } + invoke('deprecate', ...arguments); } diff --git a/packages/ember-debug/lib/warn.js b/packages/ember-debug/lib/warn.js index bf8e7366365..322491d45df 100644 --- a/packages/ember-debug/lib/warn.js +++ b/packages/ember-debug/lib/warn.js @@ -1,3 +1,4 @@ +import Ember from 'ember-metal/core'; import Logger from 'ember-metal/logger'; import { registerHandler as genericRegisterHandler, invoke } from 'ember-debug/handlers'; @@ -12,6 +13,11 @@ registerHandler(function logWarning(message, options) { } }); +export let missingOptionsDeprecation = 'When calling `Ember.warn` you ' + + 'must provide an `options` hash as the third parameter. ' + + '`options` should include an `id` property.'; +export let missingOptionsIdDeprecation = 'When calling `Ember.warn` you must provide `id` in options.'; + /** Display a warning with the provided message. Ember build tools will remove any calls to `Ember.warn()` when doing a production build. @@ -22,6 +28,22 @@ registerHandler(function logWarning(message, options) { will be displayed. @public */ -export default function warn() { +export default function warn(message, test, options) { + if (!options) { + Ember.deprecate( + missingOptionsDeprecation, + false, + { id: 'ember-debug.warn-options-missing', until: '3.0.0' } + ); + } + + if (options && !options.id) { + Ember.deprecate( + missingOptionsIdDeprecation, + false, + { id: 'ember-debug.warn-id-missing', until: '3.0.0' } + ); + } + invoke('warn', ...arguments); } diff --git a/packages/ember-debug/tests/main_test.js b/packages/ember-debug/tests/main_test.js index 92f9c32673c..95271847d99 100644 --- a/packages/ember-debug/tests/main_test.js +++ b/packages/ember-debug/tests/main_test.js @@ -1,6 +1,17 @@ import Ember from 'ember-metal/core'; import { HANDLERS } from 'ember-debug/handlers'; -import { registerHandler } from 'ember-debug/deprecate'; +import { + registerHandler, + missingOptionsDeprecation, + missingOptionsIdDeprecation, + missingOptionsUntilDeprecation +} from 'ember-debug/deprecate'; + +import { + missingOptionsIdDeprecation as missingWarnOptionsIdDeprecation, + missingOptionsDeprecation as missingWarnOptionsDeprecation, + registerHandler as registerWarnHandler +} from 'ember-debug/warn'; let originalEnvValue; let originalDeprecateHandler; @@ -26,7 +37,7 @@ QUnit.test('Ember.deprecate does not throw if RAISE_ON_DEPRECATION is false', fu Ember.ENV.RAISE_ON_DEPRECATION = false; try { - Ember.deprecate('Should not throw', false); + Ember.deprecate('Should not throw', false, { id: 'test', until: 'forever' }); assert.ok(true, 'Ember.deprecate did not throw'); } catch(e) { assert.ok(false, `Expected Ember.deprecate not to throw but it did: ${e.message}`); @@ -39,7 +50,7 @@ QUnit.test('Ember.deprecate re-sets deprecation level to RAISE if ENV.RAISE_ON_D Ember.ENV.RAISE_ON_DEPRECATION = false; try { - Ember.deprecate('Should not throw', false); + Ember.deprecate('Should not throw', false, { id: 'test', until: 'forever' }); assert.ok(true, 'Ember.deprecate did not throw'); } catch(e) { assert.ok(false, `Expected Ember.deprecate not to throw but it did: ${e.message}`); @@ -48,7 +59,7 @@ QUnit.test('Ember.deprecate re-sets deprecation level to RAISE if ENV.RAISE_ON_D Ember.ENV.RAISE_ON_DEPRECATION = true; assert.throws(function() { - Ember.deprecate('Should throw', false); + Ember.deprecate('Should throw', false, { id: 'test', until: 'forever' }); }, /Should throw/); }); @@ -63,18 +74,18 @@ QUnit.test('When ENV.RAISE_ON_DEPRECATION is true, it is still possible to silen }); try { - Ember.deprecate('should be silenced with matching id', false, { id: 'my-deprecation' }); + Ember.deprecate('should be silenced with matching id', false, { id: 'my-deprecation', until: 'forever' }); assert.ok(true, 'Did not throw when level is set by id'); } catch(e) { assert.ok(false, `Expected Ember.deprecate not to throw but it did: ${e.message}`); } assert.throws(function() { - Ember.deprecate('Should throw with no id', false); - }, /Should throw with no id/); + Ember.deprecate('Should throw with no matching id', false, { id: 'test', until: 'forever' }); + }, /Should throw with no matching id/); assert.throws(function() { - Ember.deprecate('Should throw with non-matching id', false, { id: 'other-id' }); + Ember.deprecate('Should throw with non-matching id', false, { id: 'other-id', until: 'forever' }); }, /Should throw with non-matching id/); }); @@ -82,15 +93,15 @@ QUnit.test('Ember.deprecate throws deprecation if second argument is falsy', fun expect(3); throws(function() { - Ember.deprecate('Deprecation is thrown', false); + Ember.deprecate('Deprecation is thrown', false, { id: 'test', until: 'forever' }); }); throws(function() { - Ember.deprecate('Deprecation is thrown', ''); + Ember.deprecate('Deprecation is thrown', '', { id: 'test', until: 'forever' }); }); throws(function() { - Ember.deprecate('Deprecation is thrown', 0); + Ember.deprecate('Deprecation is thrown', 0, { id: 'test', until: 'forever' }); }); }); @@ -99,7 +110,7 @@ QUnit.test('Ember.deprecate does not throw deprecation if second argument is a f Ember.deprecate('Deprecation is thrown', function() { return true; - }); + }, { id: 'test', until: 'forever' }); ok(true, 'deprecation was not thrown'); }); @@ -109,16 +120,16 @@ QUnit.test('Ember.deprecate throws if second argument is a function and it retur throws(function() { Ember.deprecate('Deprecation is thrown', function() { return false; - }); + }, { id: 'test', until: 'forever' }); }); }); QUnit.test('Ember.deprecate does not throw deprecations if second argument is truthy', function() { expect(1); - Ember.deprecate('Deprecation is thrown', true); - Ember.deprecate('Deprecation is thrown', '1'); - Ember.deprecate('Deprecation is thrown', 1); + Ember.deprecate('Deprecation is thrown', true, { id: 'test', until: 'forever' }); + Ember.deprecate('Deprecation is thrown', '1', { id: 'test', until: 'forever' }); + Ember.deprecate('Deprecation is thrown', 1, { id: 'test', until: 'forever' }); ok(true, 'deprecations were not thrown'); }); @@ -181,6 +192,7 @@ QUnit.test('Ember.assert does not throw if second argument is an object', functi QUnit.test('Ember.deprecate does not throw a deprecation at log and silence levels', function() { expect(4); let id = 'ABC'; + let until = 'forever'; let shouldThrow = false; registerHandler(function(message, options, next) { @@ -192,14 +204,14 @@ QUnit.test('Ember.deprecate does not throw a deprecation at log and silence leve }); try { - Ember.deprecate('Deprecation for testing purposes', false, { id }); + Ember.deprecate('Deprecation for testing purposes', false, { id, until }); ok(true, 'Deprecation did not throw'); } catch(e) { ok(false, 'Deprecation was thrown despite being added to blacklist'); } try { - Ember.deprecate('Deprecation for testing purposes', false, { id }); + Ember.deprecate('Deprecation for testing purposes', false, { id, until }); ok(true, 'Deprecation did not throw'); } catch(e) { ok(false, 'Deprecation was thrown despite being added to blacklist'); @@ -208,12 +220,83 @@ QUnit.test('Ember.deprecate does not throw a deprecation at log and silence leve shouldThrow = true; throws(function() { - Ember.deprecate('Deprecation is thrown', false, { id }); + Ember.deprecate('Deprecation is thrown', false, { id, until }); }); throws(function() { - Ember.deprecate('Deprecation is thrown', false, { id }); + Ember.deprecate('Deprecation is thrown', false, { id, until }); + }); +}); + +QUnit.test('Ember.deprecate without options triggers a deprecation', function(assert) { + assert.expect(4); + + registerHandler(function(message) { + if (message === missingOptionsDeprecation) { + assert.ok(true, 'proper deprecation is triggered when options is missing'); + } else if (message === 'foo') { + assert.ok(true, 'original deprecation is still triggered'); + } + }); + + Ember.deprecate('foo'); + Ember.deprecate('foo', false, { }); +}); + +QUnit.test('Ember.deprecate without options.id triggers a deprecation', function(assert) { + assert.expect(2); + + registerHandler(function(message) { + if (message === missingOptionsIdDeprecation) { + assert.ok(true, 'proper deprecation is triggered when options.id is missing'); + } else if (message === 'foo') { + assert.ok(true, 'original deprecation is still triggered'); + } + }); + + Ember.deprecate('foo', false, { until: 'forever' }); +}); + +QUnit.test('Ember.deprecate without options.until triggers a deprecation', function(assert) { + assert.expect(2); + + registerHandler(function(message) { + if (message === missingOptionsUntilDeprecation) { + assert.ok(true, 'proper deprecation is triggered when options.until is missing'); + } else if (message === 'foo') { + assert.ok(true, 'original deprecation is still triggered'); + } + }); + + Ember.deprecate('foo', false, { id: 'test' }); +}); + +QUnit.test('Ember.warn without options triggers a deprecation', function(assert) { + assert.expect(2); + + registerHandler(function(message) { + assert.equal(message, missingWarnOptionsDeprecation, 'deprecation is triggered when options is missing'); + }); + + registerWarnHandler(function(message) { + assert.equal(message, 'foo', 'original warning is triggered'); }); + + Ember.warn('foo'); +}); + +QUnit.test('Ember.warn without options.id triggers a deprecation', function(assert) { + assert.expect(2); + + registerHandler(function(message) { + assert.equal(message, missingWarnOptionsIdDeprecation, 'deprecation is triggered when options is missing'); + }); + + registerWarnHandler(function(message) { + assert.equal(message, 'foo', 'original warning is triggered'); + }); + + Ember.warn('foo', false, { }); });