Merge pull request #12370 from Kuzirashi/master

[BUGFIX canary] Deprecate passing function as test argument to deprecate/warn/assert

packages/ember-application/lib/utils/validate-type.js

@@ -5,7 +5,7 @@
 
 import { assert, deprecate } from 'ember-metal/debug';
 
-let VALIDATED_TYPES = {
+const VALIDATED_TYPES = {
   route:     ['assert',    'isRouteFactory',     'Ember.Route'],
   component: ['deprecate', 'isComponentFactory', 'Ember.Component'],
   view:      ['deprecate', 'isViewFactory',      'Ember.View'],
@@ -27,16 +27,14 @@ export default function validateType(resolvedType, parsedName) {
       `property set to true. You registered ${resolvedType} as a ${parsedName.type} ` +
       `factory. Either add the \`${factoryFlag}\` property to this factory or ` +
       `extend from ${expectedType}.`,
-      resolvedType[factoryFlag],
+      !!resolvedType[factoryFlag],
       { id: 'ember-application.validate-type', until: '3.0.0' }
     );
   } else {
     assert(
       `Expected ${parsedName.fullName} to resolve to an ${expectedType} but ` +
       `instead it was ${resolvedType}.`,
-      function() {
-        return resolvedType[factoryFlag];
-      }
+      !!resolvedType[factoryFlag]
     );
   }
 }

packages/ember-debug/lib/deprecate.js

@@ -85,9 +85,8 @@ export let missingOptionsUntilDeprecation = 'When calling `Ember.deprecate` you
 
   @method deprecate
   @param {String} message A description of 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 {Boolean} test A boolean. If falsy, the deprecation
+    will be displayed.
   @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

packages/ember-debug/lib/handlers.js

@@ -1,9 +1,26 @@
 import isPlainFunction from 'ember-debug/is-plain-function';
+import deprecate from 'ember-debug/deprecate';
 
 export let HANDLERS = { };
 
-function normalizeTest(test) {
-  return isPlainFunction(test) ? test() : test;
+export function generateTestAsFunctionDeprecation(source) {
+  return `Calling \`${source}\` with a function argument is deprecated. Please ` +
+    `use \`!!Constructor\` for constructors, or an \`IIFE\` to compute the test for deprecation. ` +
+    `In a future version functions will be treated as truthy values instead of being executed.`;
+}
+
+function normalizeTest(test, source) {
+  if (isPlainFunction(test)) {
+    deprecate(
+      generateTestAsFunctionDeprecation(source),
+      false,
+      { id: 'ember-debug.deprecate-test-as-function', until: '2.5.0' }
+    );
+
+    return test();
+  }
+
+  return test;
 }
 
 export function registerHandler(type, callback) {
@@ -15,7 +32,7 @@ export function registerHandler(type, callback) {
 }
 
 export function invoke(type, message, test, options) {
-  if (normalizeTest(test)) { return; }
+  if (normalizeTest(test, 'Ember.' + type)) { return; }
 
   let handlerForType = HANDLERS[type];
 

packages/ember-debug/lib/main.js

@@ -16,6 +16,7 @@ import _warn, {
   registerHandler as registerWarnHandler
 } from 'ember-debug/warn';
 import isPlainFunction from 'ember-debug/is-plain-function';
+import { generateTestAsFunctionDeprecation } from 'ember-debug/handlers';
 
 /**
 @module ember
@@ -44,15 +45,20 @@ import isPlainFunction from 'ember-debug/is-plain-function';
   @method assert
   @param {String} desc A description of the assertion. This will become
     the text of the Error thrown if the assertion fails.
-  @param {Boolean|Function} test Must be truthy for the assertion to pass. If
-    falsy, an exception will be thrown. If this is a function, it will be executed and
-    its return value will be used as condition.
+  @param {Boolean} test Must be truthy for the assertion to pass. If
+    falsy, an exception will be thrown.
   @public
 */
 setDebugFunction('assert', function assert(desc, test) {
-  var throwAssertion;
+  let throwAssertion;
 
   if (isPlainFunction(test)) {
+    deprecate(
+      generateTestAsFunctionDeprecation('Ember.assert'),
+      false,
+      { id: 'ember-debug.deprecate-test-as-function', until: '2.5.0' }
+    );
+
     throwAssertion = !test();
   } else {
     throwAssertion = !test;

packages/ember-debug/tests/main_test.js

@@ -1,6 +1,6 @@
 import Ember from 'ember-metal/core';
 import EmberObject from 'ember-runtime/system/object';
-import { HANDLERS } from 'ember-debug/handlers';
+import { HANDLERS, generateTestAsFunctionDeprecation } from 'ember-debug/handlers';
 import {
   registerHandler,
   missingOptionsDeprecation,
@@ -13,6 +13,7 @@ import {
   missingOptionsDeprecation as missingWarnOptionsDeprecation,
   registerHandler as registerWarnHandler
 } from 'ember-debug/warn';
+import deprecate from 'ember-debug/deprecate';
 
 let originalEnvValue;
 let originalDeprecateHandler;
@@ -106,14 +107,12 @@ QUnit.test('Ember.deprecate throws deprecation if second argument is falsy', fun
   });
 });
 
-QUnit.test('Ember.deprecate does not throw deprecation if second argument is a function and it returns true', function() {
-  expect(1);
-
-  Ember.deprecate('Deprecation is thrown', function() {
-    return true;
-  }, { id: 'test', until: 'forever' });
+QUnit.test('Ember.deprecate throws deprecation if second argument is a function and it returns true', function(assert) {
+  assert.expect(1);
 
-  ok(true, 'deprecation was not thrown');
+  throws(() => {
+    Ember.deprecate('This deprecation is not thrown, but argument deprecation is thrown', () => true, { id: 'test', until: 'forever' });
+  });
 });
 
 QUnit.test('Ember.deprecate throws if second argument is a function and it returns false', function() {
@@ -151,14 +150,14 @@ QUnit.test('Ember.assert throws if second argument is falsy', function() {
   });
 });
 
-QUnit.test('Ember.assert does not throw if second argument is a function and it returns true', function() {
-  expect(1);
-
-  Ember.assert('Assertion is thrown', function() {
-    return true;
-  });
+QUnit.test('Ember.assert does not throw if second argument is a function and it returns true', function(assert) {
+  assert.expect(1);
 
-  ok(true, 'assertion was not thrown');
+  // shouldn't trigger an assertion, but deprecation from using function as test is expected
+  expectDeprecation(
+    () => Ember.assert('Assertion is thrown', () => true),
+    generateTestAsFunctionDeprecation('Ember.assert')
+  );
 });
 
 QUnit.test('Ember.assert throws if second argument is a function and it returns false', function() {
@@ -182,7 +181,7 @@ QUnit.test('Ember.assert does not throw if second argument is truthy', function(
 
 QUnit.test('Ember.assert does not throw if second argument is an object', function() {
   expect(1);
-  var Igor = EmberObject.extend();
+  let Igor = EmberObject.extend();
 
   Ember.assert('is truthy', Igor);
   Ember.assert('is truthy', Igor.create());
@@ -224,8 +223,6 @@ QUnit.test('Ember.deprecate does not throw a deprecation at log and silence leve
     Ember.deprecate('Deprecation is thrown', false, { id, until });
   });
 
-
-
   throws(function() {
     Ember.deprecate('Deprecation is thrown', false, { id, until });
   });
@@ -301,3 +298,39 @@ QUnit.test('Ember.warn without options.id triggers a deprecation', function(asse
 
   Ember.warn('foo', false, { });
 });
+
+QUnit.test('Ember.deprecate triggers a deprecation when test argument is a function', function(assert) {
+  assert.expect(1);
+
+  registerHandler(message => assert.equal(
+    message,
+    generateTestAsFunctionDeprecation('Ember.deprecate'),
+    'proper deprecation is triggered when test argument is a function'
+  ));
+
+  deprecate('Deprecation is thrown', () => true, { id: 'test', until: 'forever' });
+});
+
+QUnit.test('Ember.warn triggers a deprecation when test argument is a function', function(assert) {
+  assert.expect(1);
+
+  registerHandler(message => assert.equal(
+    message,
+    generateTestAsFunctionDeprecation('Ember.warn'),
+    'proper deprecation is triggered when test argument is a function'
+  ));
+
+  Ember.warn('Warning is thrown', () => true, { id: 'test' });
+});
+
+QUnit.test('Ember.assert triggers a deprecation when test argument is a function', function(assert) {
+  assert.expect(1);
+
+  registerHandler(message => assert.equal(
+    message,
+    generateTestAsFunctionDeprecation('Ember.assert'),
+    'proper deprecation is triggered when test argument is a function'
+  ));
+
+  Ember.assert('Assertion is thrown', () => true);
+});

packages/ember-htmlbars/lib/node-managers/view-node-manager.js

@@ -27,13 +27,13 @@ function ViewNodeManager(component, scope, renderNode, block, expectElement) {
 export default ViewNodeManager;
 
 ViewNodeManager.create = function ViewNodeManager_create(renderNode, env, attrs, found, parentView, path, contentScope, contentTemplate) {
-  assert('HTMLBars error: Could not find component named "' + path + '" (no component or template with that name was found)', function() {
+  assert('HTMLBars error: Could not find component named "' + path + '" (no component or template with that name was found)', !!(function() {
     if (path) {
       return found.component || found.layout;
     } else {
       return found.component || found.layout || contentTemplate;
     }
-  });
+  }()));
 
   var component;
   var componentInfo = { layout: found.layout };

packages/ember-htmlbars/tests/node-managers/view-node-manager-test.js

@@ -0,0 +1,63 @@
+import ViewNodeManager from 'ember-htmlbars/node-managers/view-node-manager';
+
+QUnit.module('ember-htmlbars: node-managers - ViewNodeManager');
+
+QUnit.test('create method should assert if component hasn\'t been found', assert => {
+  assert.expect(1);
+
+  let found = {
+    component: null,
+    layout: null
+  };
+
+  let path;
+
+  expectAssertion(() => {
+    ViewNodeManager.create(null, null, null, found, null, path);
+  }, 'HTMLBars error: Could not find component named "' + path + '" (no component or template with that name was found)');
+});
+
+QUnit.test('create method shouldn\'t assert if `found.component` is truthy', assert => {
+  assert.expect(1);
+
+  let found = {
+    component: {},
+    layout: null
+  };
+  let attrs = {};
+  let renderNode = {};
+
+  let env = {
+    renderer: {
+      componentUpdateAttrs() {
+        assert.ok('env.renderer.componentUpdateAttrs called');
+      }
+    }
+  };
+
+  ViewNodeManager.create(renderNode, env, attrs, found);
+});
+
+QUnit.test('create method shouldn\'t assert if `found.layout` is truthy', assert => {
+  assert.expect(0);
+
+  let found = {
+    component: null,
+    layout: true
+  };
+
+  ViewNodeManager.create(null, null, null, found);
+});
+
+QUnit.test('create method shouldn\'t assert if `path` is falsy and `contentTemplate` is truthy', assert => {
+  assert.expect(0);
+
+  let found = {
+    component: null,
+    layout: null
+  };
+  let path = null;
+  let contentTemplate = true;
+
+  ViewNodeManager.create(null, null, null, found, null, path, null, contentTemplate);
+});

packages/ember-template-compiler/lib/plugins/assert-no-view-and-controller-paths.js

@@ -53,8 +53,7 @@ function assertPaths(moduleName, node, paths) {
 
 function assertPath(moduleName, node, path) {
   assert(
-    `Using \`{{${path && path.type === 'PathExpression' && path.parts[0]}}}\` or any path based on it ${calculateLocationDisplay(moduleName, node.loc)}has been removed in Ember 2.0`,
-    function assertPath_test() {
+    `Using \`{{${path && path.type === 'PathExpression' && path.parts[0]}}}\` or any path based on it ${calculateLocationDisplay(moduleName, node.loc)}has been removed in Ember 2.0`, () => {
       let noAssertion = true;
 
       const viewKeyword = path && path.type === 'PathExpression' && path.parts && path.parts[0];
@@ -65,7 +64,7 @@ function assertPath(moduleName, node, path) {
       }
 
       return noAssertion;
-    }, {
+    }(), {
       id: (path.parts && path.parts[0] === 'view' ? 'view.keyword.view' : 'view.keyword.controller'),
       until: '2.0.0'
     }

packages/ember-views/lib/system/build-component-template.js

@@ -294,8 +294,8 @@ function normalizeClasses(classes, output, streamBasePath) {
 }
 
 function validateTaglessComponent(component) {
-  assert('You cannot use `classNameBindings` on a tag-less component: ' + component.toString(), function() {
+  assert('You cannot use `classNameBindings` on a tag-less component: ' + component.toString(), () => {
     var classNameBindings = component.classNameBindings;
     return !classNameBindings || classNameBindings.length === 0;
-  });
+  }());
 }

packages/ember-views/lib/views/container_view.js

@@ -245,16 +245,16 @@ var ContainerView = View.extend(MutableArray, {
   layout: containerViewTemplate,
 
   replace(idx, removedCount, addedViews=[]) {
-    var addedCount = get(addedViews, 'length');
-    var childViews = get(this, 'childViews');
+    let addedCount = get(addedViews, 'length');
+    let childViews = get(this, 'childViews');
 
     assert('You can\'t add a child to a container - the child is already a child of another view', () => {
-      for (var i = 0, l = addedViews.length; i < l; i++) {
-        var item = addedViews[i];
+      for (let i = 0, l = addedViews.length; i < l; i++) {
+        let item = addedViews[i];
         if (item.parentView && item.parentView !== this) { return false; }
       }
       return true;
-    });
+    }());
 
     this.arrayContentWillChange(idx, removedCount, addedCount);
 
@@ -266,7 +266,7 @@ var ContainerView = View.extend(MutableArray, {
     // Because of this, we synchronously fix up the parentView/childViews tree
     // as soon as views are added or removed, despite the fact that this will
     // happen automatically when we render.
-    var removedViews = childViews.slice(idx, idx + removedCount);
+    let removedViews = childViews.slice(idx, idx + removedCount);
     removedViews.forEach(view => this.unlinkChild(view));
     addedViews.forEach(view => this.linkChild(view));