catch ambiguous step definitions

closes #442
resolves #176

features/ambiguous_step.feature

@@ -0,0 +1,33 @@
+Feature: Ambiguous Steps
+
+  Scenario:
+    Given a file named "features/a.feature" with:
+      """
+      Feature:
+        Scenario:
+          Given a ambiguous step
+      """
+    Given a file named "features/step_definitions/cucumber_steps.js" with:
+      """
+      var cucumberSteps = function() {
+        this.When(/^a ambiguous step$/, function() { });
+        this.When(/^a (.*) step$/, function(status) { });
+      };
+      module.exports = cucumberSteps;
+      """
+    When I run cucumber.js with `-f progress`
+    Then it outputs this text:
+      """
+      A
+
+      1 scenario (1 ambiguous)
+      1 step (1 ambiguous)
+      <duration-stat>
+
+      The following steps have multiple matching definitions:
+
+      "a ambiguous step" matches:
+      /^a ambiguous step$/ # features/step_definitions/cucumber_steps.js:2
+      /^a (.*) step$/      # features/step_definitions/cucumber_steps.js:3
+      """
+    And the exit status should be 1

features/core.feature

@@ -113,17 +113,17 @@ Feature: Core feature elements execution
           this.parameters = {};
         };
 
-        this.When(/^I call a step with "(.*)"$/, function(arg) {
+        this.When(/^I call a step with "([^"]*)"$/, function(arg) {
           this.parameters['1'] = arg;
         });
 
-        this.When(/^I call a step with "(.*)", "(.*)" and "(.*)"$/, function(arg1, arg2, arg3) {
+        this.When(/^I call a step with "([^"]*)", "([^"]*)" and "([^"]*)"$/, function(arg1, arg2, arg3) {
           this.parameters['1'] = arg1;
           this.parameters['2'] = arg2;
           this.parameters['3'] = arg3;
         })
 
-        this.Then(/^the (\d+)(?:st|nd|rd) received parameter should be "(.*)"$/, function(index, arg){
+        this.Then(/^the (\d+)(?:st|nd|rd) received parameter should be "([^"]*)"$/, function(index, arg){
           assert.equal(this.parameters[index], arg);
         });
       };

lib/cucumber/ast/hook_step.js

@@ -11,12 +11,12 @@ function HookStep(keyword) {
     return false;
   };
 
-  self.setHook = function setHook(newHook) {
-    hook = newHook;
+  self.getHook = function getHook() {
+    return hook;
   };
 
-  self.getStepDefinition = function getStepDefinition() {
-    return hook;
+  self.setHook = function setHook(newHook) {
+    hook = newHook;
   };
 
   return self;

lib/cucumber/ast/step.js

@@ -141,24 +141,6 @@ function Step(keyword, name, uri, line) {
         result = previousStep.isEventStep();
       }
       return result;
-    },
-
-    acceptVisitor: function acceptVisitor(visitor, callback) {
-      self.execute(visitor, function (stepResult) {
-        visitor.visitStepResult(stepResult, callback);
-      });
-    },
-
-    getStepDefinition: function getStepDefinition(visitor) {
-      return visitor.lookupStepDefinitionByName(name);
-    },
-
-    execute: function execute(visitor, callback) {
-      var stepDefinition = self.getStepDefinition(visitor);
-      var world          = visitor.getWorld();
-      var scenario       = visitor.getScenario();
-      var defaultTimeout = visitor.getDefaultTimeout();
-      stepDefinition.invoke(self, world, scenario, defaultTimeout, callback);
     }
   };
   return self;

lib/cucumber/listener/progress_formatter.js

@@ -21,6 +21,7 @@ function ProgressFormatter(options) {
   };
 
   var characters = {};
+  characters[Cucumber.Status.AMBIGUOUS] = 'A';
   characters[Cucumber.Status.FAILED] = 'F';
   characters[Cucumber.Status.PASSED] = '.';
   characters[Cucumber.Status.PENDING] = 'P';

lib/cucumber/listener/stats_journal.js

@@ -4,6 +4,7 @@ function StatsJournal(options) {
 
   function getCountsObject () {
     var statuses = [
+      Cucumber.Status.AMBIGUOUS,
       Cucumber.Status.FAILED,
       Cucumber.Status.PASSED,
       Cucumber.Status.PENDING,

lib/cucumber/listener/summary_formatter.js

@@ -1,9 +1,11 @@
 function SummaryFormatter(options) {
   var Cucumber = require('../../cucumber');
   var Duration = require('duration');
+  var Table    = require('cli-table');
   var path     = require('path');
   var _        = require('lodash');
 
+  var ambiguousStepLogBuffer = '';
   var failedScenarioLogBuffer = '';
   var failedStepResultLogBuffer = '';
   var undefinedStepLogBuffer = '';
@@ -12,6 +14,7 @@ function SummaryFormatter(options) {
   var statusReportOrder = [
     Cucumber.Status.FAILED,
     Cucumber.Status.UNDEFINED,
+    Cucumber.Status.AMBIGUOUS,
     Cucumber.Status.PENDING,
     Cucumber.Status.SKIPPED,
     Cucumber.Status.PASSED
@@ -29,23 +32,20 @@ function SummaryFormatter(options) {
   self.handleStepResultEvent = function handleStepResult(event, callback) {
     var stepResult = event.getPayloadItem('stepResult');
     var status = stepResult.getStatus();
-    if (status === Cucumber.Status.UNDEFINED) {
-      self.handleUndefinedStepResult(stepResult);
-    } else if (status === Cucumber.Status.FAILED) {
-      self.handleFailedStepResult(stepResult);
+    switch (status) {
+      case Cucumber.Status.AMBIGUOUS:
+        self.storeAmbiguousStepResult(stepResult);
+        break;
+      case Cucumber.Status.FAILED:
+        self.storeFailedStepResult(stepResult);
+        break;
+      case Cucumber.Status.UNDEFINED:
+        self.storeUndefinedStepResult(stepResult);
+        break;
     }
     callback();
   };
 
-  self.handleUndefinedStepResult = function handleUndefinedStepResult(stepResult) {
-    var step = stepResult.getStep();
-    self.storeUndefinedStepResult(step);
-  };
-
-  self.handleFailedStepResult = function handleFailedStepResult(stepResult) {
-    self.storeFailedStepResult(stepResult);
-  };
-
   self.handleAfterScenarioEvent = function handleAfterScenarioEvent(event, callback) {
     if (statsJournal.isCurrentScenarioFailing()) {
       var scenario = event.getPayloadItem('scenario');
@@ -59,6 +59,33 @@ function SummaryFormatter(options) {
     self.finish(callback);
   };
 
+  self.storeAmbiguousStepResult = function storeAmbiguousStepResult(stepResult) {
+    var step = stepResult.getStep();
+    var stepDefinitions = stepResult.getAmbiguousStepDefinitions();
+
+    var table = new Table({
+      chars: {
+        'bottom': '', 'bottom-left': '', 'bottom-mid': '', 'bottom-right': '',
+        'left': '', 'left-mid': '',
+        'mid': '', 'mid-mid': '',
+        'middle': ' ',
+        'right': '', 'right-mid': '',
+        'top': '' , 'top-left': '', 'top-mid': '', 'top-right': ''
+      },
+      style: {
+        'padding-left': 0, 'padding-right': 0
+      }
+    });
+    table.push.apply(table, stepDefinitions.map(function (stepDefinition) {
+      var pattern = stepDefinition.getPattern();
+      var relativeUri = path.relative(process.cwd(), stepDefinition.getUri());
+      var line = stepDefinition.getLine();
+      return [colors.ambiguous(pattern), colors.comment('# ' + relativeUri + ':' + line)];
+    }));
+    var message = colors.ambiguous('"' + step.getName() + '" matches:') + '\n' + table.toString();
+    self.appendStringToAmbiguousStepLogBuffer(message);
+  };
+
   self.storeFailedStepResult = function storeFailedStepResult(failedStepResult) {
     var failureException = failedStepResult.getFailureException();
     var failureMessage = failureException.stack || failureException;
@@ -72,12 +99,18 @@ function SummaryFormatter(options) {
     self.appendStringToFailedScenarioLogBuffer(relativeUri + ':' + line + ' # Scenario: ' + name);
   };
 
-  self.storeUndefinedStepResult = function storeUndefinedStepResult(step) {
+  self.storeUndefinedStepResult = function storeUndefinedStepResult(stepResult) {
+    var step = stepResult.getStep();
     var snippetBuilder = Cucumber.SupportCode.StepDefinitionSnippetBuilder(step, options.snippetSyntax);
     var snippet        = snippetBuilder.buildSnippet();
     self.appendStringToUndefinedStepLogBuffer(snippet);
   };
 
+  self.appendStringToAmbiguousStepLogBuffer = function appendStringToAmbiguousStepLogBuffer(string) {
+    if (ambiguousStepLogBuffer.indexOf(string) === -1)
+      ambiguousStepLogBuffer += string + '\n\n';
+  };
+
   self.appendStringToFailedScenarioLogBuffer = function appendStringToFailedScenarioLogBuffer(string) {
     failedScenarioLogBuffer += string + '\n';
   };
@@ -115,6 +148,13 @@ function SummaryFormatter(options) {
     self.logDuration();
     if (undefinedStepLogBuffer)
       self.logUndefinedStepSnippets();
+    if (ambiguousStepLogBuffer)
+      self.logAmbiguousSteps();
+  };
+
+  self.logAmbiguousSteps = function logAmbiguousSteps() {
+    self.log(colors.ambiguous('\nThe following steps have multiple matching definitions:\n\n'));
+    self.log(colors.ambiguous(ambiguousStepLogBuffer));
   };
 
   self.logFailedStepResults = function logFailedStepResults() {

lib/cucumber/runtime/ast_tree_walker.js

@@ -105,14 +105,14 @@ function AstTreeWalker(features, supportCodeLibrary, listeners, options) {
     visitBeforeSteps: function visitBeforeSteps(callback) {
       beforeSteps.asyncForEach(function (beforeStep, callback) {
         self.witnessHook();
-        beforeStep.acceptVisitor(self, callback);
+        self.executeHookStep(beforeStep, callback);
       }, callback);
     },
 
     visitAfterSteps: function visitAfterSteps(callback) {
       afterSteps.asyncForEach(function (afterStep, callback) {
         self.witnessHook();
-        afterStep.acceptVisitor(self, callback);
+        self.executeHookStep(afterStep, callback);
       }, callback);
     },
 
@@ -179,10 +179,6 @@ function AstTreeWalker(features, supportCodeLibrary, listeners, options) {
       broadcastToListeners(listeners, onRuntimeListenersComplete);
     },
 
-    lookupStepDefinitionByName: function lookupStepDefinitionByName(stepName) {
-      return supportCodeLibrary.lookupStepDefinitionByName(stepName);
-    },
-
     setWorld: function setWorld(newWorld) {
       world = newWorld;
     },
@@ -195,11 +191,6 @@ function AstTreeWalker(features, supportCodeLibrary, listeners, options) {
       return supportCodeLibrary.getDefaultTimeout();
     },
 
-    isStepUndefined: function isStepUndefined(step) {
-      var stepName = step.getName();
-      return !supportCodeLibrary.isStepDefinitionNameDefined(stepName);
-    },
-
     getScenarioStatus: function getScenarioStatus() {
       return scenarioResult.getStatus();
     },
@@ -240,17 +231,40 @@ function AstTreeWalker(features, supportCodeLibrary, listeners, options) {
     },
 
     processStep: function processStep(step, callback) {
-      if (self.isStepUndefined(step)) {
+      var stepName = step.getName();
+      var stepDefinitions = supportCodeLibrary.lookupStepDefinitionsByName(stepName);
+      if (stepDefinitions.length === 0) {
         self.skipUndefinedStep(step, callback);
+      } else if (stepDefinitions.length > 1) {
+        self.skipAmbiguousStep(step, stepDefinitions, callback);
       } else if (options.dryRun || self.isSkippingSteps()) {
         self.skipStep(step, callback);
       } else {
-        self.executeStep(step, callback);
+        self.executeStep(step, stepDefinitions[0], callback);
       }
     },
 
-    executeStep: function executeStep(step, callback) {
-      step.acceptVisitor(self, callback);
+    executeHookStep: function executeHook(hookStep, callback) {
+      var stepDefinition = hookStep.getHook();
+      self.executeStep(hookStep, stepDefinition, callback);
+    },
+
+    executeStep: function executeStep(step, stepDefinition, callback) {
+      var world          = self.getWorld();
+      var scenario       = self.getScenario();
+      var defaultTimeout = self.getDefaultTimeout();
+      stepDefinition.invoke(step, world, scenario, defaultTimeout, function (stepResult) {
+        self.visitStepResult(stepResult, callback);
+      });
+    },
+
+    skipAmbiguousStep: function skipAmbiguousStep(step, stepDefinitions, callback) {
+      var ambiguousStepResult = Cucumber.Runtime.StepResult({
+        ambiguousStepDefinitions: stepDefinitions,
+        step: step,
+        status: Cucumber.Status.AMBIGUOUS
+      });
+      self.visitStepResult(ambiguousStepResult, callback);
     },
 
     skipStep: function skipStep(step, callback) {

lib/cucumber/runtime/features_result.js

@@ -11,6 +11,7 @@ function FeaturesResult(strict) {
     witnessStepResult: function witnessStepResult(stepResult) {
       var stepStatus = stepResult.getStatus();
       switch (stepStatus) {
+        case Cucumber.Status.AMBIGUOUS:
         case Cucumber.Status.FAILED:
           success = false;
           break;

lib/cucumber/runtime/scenario_result.js

@@ -8,6 +8,7 @@ function ScenarioResult() {
     switch (stepStatus) {
       case Cucumber.Status.FAILED:
         return true;
+      case Cucumber.Status.AMBIGUOUS:
       case Cucumber.Status.PENDING:
       case Cucumber.Status.SKIPPED:
       case Cucumber.Status.UNDEFINED:

lib/cucumber/runtime/step_result.js

@@ -1,5 +1,9 @@
 function StepResult(payload) {
   var self = {
+    getAmbiguousStepDefinitions: function getAmbiguousStepDefinitions() {
+      return payload.ambiguousStepDefinitions;
+    },
+
     getAttachments: function getAttachments() {
       return payload.attachments;
     },

lib/cucumber/status.js

@@ -1,5 +1,6 @@
 var Status = {};
 
+Status.AMBIGUOUS = 'ambiguous';
 Status.FAILED = 'failed';
 Status.PENDING = 'pending';
 Status.PASSED = 'passed';

lib/cucumber/support_code/library.js

@@ -1,5 +1,6 @@
 function Library(supportCodeDefinition) {
   var Cucumber = require('../../cucumber');
+  var callsite = require('callsite');
 
   var listeners        = [];
   var stepDefinitions  = [];
@@ -46,20 +47,10 @@ function Library(supportCodeDefinition) {
       });
     },
 
-    lookupStepDefinitionByName: function lookupStepDefinitionByName(name) {
-      var matchingStepDefinition;
-
-      stepDefinitions.forEach(function (stepDefinition) {
-        if (stepDefinition.matchesStepName(name)) {
-          matchingStepDefinition = stepDefinition;
-        }
+    lookupStepDefinitionsByName: function lookupStepDefinitionsByName(name) {
+      return stepDefinitions.filter(function (stepDefinition) {
+        return stepDefinition.matchesStepName(name);
       });
-      return matchingStepDefinition;
-    },
-
-    isStepDefinitionNameDefined: function isStepDefinitionNameDefined(name) {
-      var stepDefinition = self.lookupStepDefinitionByName(name);
-      return (stepDefinition !== undefined);
     },
 
     defineAroundHook: function defineAroundHook() {
@@ -88,7 +79,10 @@ function Library(supportCodeDefinition) {
         code = options;
         options = {};
       }
-      var stepDefinition = Cucumber.SupportCode.StepDefinition(name, options, code);
+      var site = callsite();
+      var line = site[1].getLineNumber();
+      var uri = site[1].getFileName();
+      var stepDefinition = Cucumber.SupportCode.StepDefinition(name, options, code, uri, line);
       stepDefinitions.push(stepDefinition);
     },