Skip to content

Commit

Permalink
Merge pull request #11851 from rwjblue/enforcement
Browse files Browse the repository at this point in the history
Enforce usage of populated options for deprecate and warn.
  • Loading branch information
rwjblue committed Jul 21, 2015
2 parents a53d497 + 733f0a7 commit ada79f2
Show file tree
Hide file tree
Showing 3 changed files with 159 additions and 24 deletions.
36 changes: 33 additions & 3 deletions packages/ember-debug/lib/deprecate.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,23 +71,53 @@ 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
`Ember.deprecate()` when doing a production build.
@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);
}
24 changes: 23 additions & 1 deletion packages/ember-debug/lib/warn.js
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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.
Expand All @@ -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);
}
123 changes: 103 additions & 20 deletions packages/ember-debug/tests/main_test.js
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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}`);
Expand All @@ -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}`);
Expand All @@ -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/);
});

Expand All @@ -63,34 +74,34 @@ 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/);
});

QUnit.test('Ember.deprecate throws deprecation if second argument is falsy', function() {
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' });
});
});

Expand All @@ -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');
});
Expand All @@ -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');
});
Expand Down Expand Up @@ -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) {
Expand All @@ -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');
Expand All @@ -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, { });
});

0 comments on commit ada79f2

Please sign in to comment.