From 9edd17fd171c40de9d61bdc82bd4e3e7eb7c7862 Mon Sep 17 00:00:00 2001
From: Sergio Arbeo <serabe@gmail.com>
Date: Tue, 15 Sep 2015 22:44:39 +0200
Subject: [PATCH] [BUGFIX beta] Change preference in positional parameters

This fixes the behaviour when a positional param conflicts conflicts with
a hash param.
---
 .../node-managers/component-node-manager.js   | 48 ++++++++++++-------
 .../integration/component_invocation_test.js  | 45 +++++++++++++++++
 2 files changed, 77 insertions(+), 16 deletions(-)

diff --git a/packages/ember-htmlbars/lib/node-managers/component-node-manager.js b/packages/ember-htmlbars/lib/node-managers/component-node-manager.js
index e496d519e1d..b080199e125 100644
--- a/packages/ember-htmlbars/lib/node-managers/component-node-manager.js
+++ b/packages/ember-htmlbars/lib/node-managers/component-node-manager.js
@@ -122,28 +122,44 @@ function processPositionalParams(renderNode, positionalParams, params, attrs) {
   // if the component is rendered via {{component}} helper, the first
   // element of `params` is the name of the component, so we need to
   // skip that when the positional parameters are constructed
-  const paramsStartIndex = renderNode.getState().isComponentHelper ? 1 : 0;
   const isNamed = typeof positionalParams === 'string';
-  let paramsStream;
 
   if (isNamed) {
-    paramsStream = new Stream(() => {
-      return readArray(params.slice(paramsStartIndex));
-    }, 'params');
+    processRestPositionalParameters(renderNode, positionalParams, params, attrs);
+  } else {
+    processNamedPositionalParameters(renderNode, positionalParams, params, attrs);
+  }
+}
 
-    attrs[positionalParams] = paramsStream;
+function processNamedPositionalParameters(renderNode, positionalParams, params, attrs) {
+  const paramsStartIndex = renderNode.getState().isComponentHelper ? 1 : 0;
+
+  for (let i = 0; i < positionalParams.length; i++) {
+    let param = params[paramsStartIndex + i];
+
+    assert(`You cannot specify both a positional param (at position ${i}) and the hash argument \`${positionalParams[i]}\`.`,
+           !(positionalParams[i] in attrs));
+
+    attrs[positionalParams[i]] = param;
   }
+}
 
-  if (isNamed) {
-    for (let i = paramsStartIndex; i < params.length; i++) {
-      let param = params[i];
-      paramsStream.addDependency(param);
-    }
-  } else {
-    for (let i = 0; i < positionalParams.length; i++) {
-      let param = params[paramsStartIndex + i];
-      attrs[positionalParams[i]] = param;
-    }
+function processRestPositionalParameters(renderNode, positionalParamsName, params, attrs) {
+  // If there is already an attribute for that variable, do nothing
+  assert(`You cannot specify positional parameters and the hash argument \`${positionalParamsName}\`.`,
+         !(positionalParamsName in attrs));
+
+  const paramsStartIndex = renderNode.getState().isComponentHelper ? 1 : 0;
+
+  let paramsStream = new Stream(() => {
+    return readArray(params.slice(paramsStartIndex));
+  }, 'params');
+
+  attrs[positionalParamsName] = paramsStream;
+
+  for (let i = paramsStartIndex; i < params.length; i++) {
+    let param = params[i];
+    paramsStream.addDependency(param);
   }
 }
 
diff --git a/packages/ember-htmlbars/tests/integration/component_invocation_test.js b/packages/ember-htmlbars/tests/integration/component_invocation_test.js
index 2adb3ff59a8..10358b00056 100644
--- a/packages/ember-htmlbars/tests/integration/component_invocation_test.js
+++ b/packages/ember-htmlbars/tests/integration/component_invocation_test.js
@@ -385,6 +385,29 @@ QUnit.test('dynamic named positional parameters', function() {
   equal(jQuery('#qunit-fixture').text(), 'Edward5');
 });
 
+QUnit.test('if a value is passed as a non-positional parameter, it takes precedence over the named one', function() {
+  var SampleComponent = Component.extend();
+  SampleComponent.reopenClass({
+    positionalParams: ['name']
+  });
+
+  registry.register('template:components/sample-component', compile('{{attrs.name}}'));
+  registry.register('component:sample-component', SampleComponent);
+
+  view = EmberView.extend({
+    layout: compile('{{sample-component notMyName name=myName}}'),
+    container: container,
+    context: {
+      myName: 'Quint',
+      notMyName: 'Sergio'
+    }
+  }).create();
+
+  expectAssertion(function() {
+    runAppend(view);
+  }, `You cannot specify both a positional param (at position 0) and the hash argument \`name\`.`);
+});
+
 QUnit.test('static arbitrary number of positional parameters', function() {
   var SampleComponent = Component.extend();
   SampleComponent.reopenClass({
@@ -406,6 +429,28 @@ QUnit.test('static arbitrary number of positional parameters', function() {
   equal(view.$('#helper').text(), 'Foo4Bar5Baz');
 });
 
+QUnit.test('arbitrary positional parameter conflict with hash parameter is reported', function() {
+  var SampleComponent = Component.extend();
+  SampleComponent.reopenClass({
+    positionalParams: 'names'
+  });
+
+  registry.register('template:components/sample-component', compile('{{#each attrs.names as |name|}}{{name}}{{/each}}'));
+  registry.register('component:sample-component', SampleComponent);
+
+  view = EmberView.extend({
+    layout: compile('{{sample-component "Foo" 4 "Bar" names=numbers id="args-3"}}'),
+    container: container,
+    context: {
+      numbers: [1, 2, 3]
+    }
+  }).create();
+
+  expectAssertion(function() {
+    runAppend(view);
+  }, `You cannot specify positional parameters and the hash argument \`names\`.`);
+});
+
 QUnit.test('dynamic arbitrary number of positional parameters', function() {
   var SampleComponent = Component.extend();
   SampleComponent.reopenClass({