cucumber / language-service

Cucumber Language Service
MIT License
18 stars 27 forks source link

Unrecognised tokens in step definition in php (Behat) files #223

Open MolbioUnige opened 4 months ago

MolbioUnige commented 4 months ago

πŸ‘“ What did you see?

LSP don't recognize context having token defined. This is a scenario I have:

      Scenario: User is an committee member
          Given user is an committee
          And I authenticate
          Then I get redirected to "/committee"     β–  Undefined step: I get redirected to "/committee"

And here the related contexts:

      /**
       * @Given user is an committee
       */
      public function userIsAnCommittee()
      {
          throw new PendingException();
      }
      /**
       * @Given I authenticate
       */
      public function iAuthenticate()
      {
          throw new PendingException();
      }
      /**
       * @Then I get redirected to :arg1
       */
     public function iGetRedirectedTo($arg1)
      {
          throw new PendingException();
      }

βœ… What did you expect to see?

The first two contexts are recognised by the lsp and I can jump to their definition, but not the third one. I tried without the surrounding double quotes but it has the same effect.

πŸ“¦ Which tool/library version are you using?

Cucumber Language Server 1.6.0

πŸ”¬ How could we reproduce it?

No response

πŸ“š Any additional context?

No response

kieran-ryan commented 4 months ago

Thanks for catching this @MolbioUnige - moved to the Cucumber Language Service project ⭐

@nirajacharya2 and @thomas-hiron, you've both recently made fantastic contributions on the php implementation; wonder whether something you could look at whether you can reproduce and additionally whether you could look at what might be a suitable fix?

thomas-hiron commented 4 months ago

Hello, I added a failing test case here: https://github.com/thomas-hiron/language-service/commit/7582299524c6630606c4984d03ef4ef80bf3be83 This looks to be caused by the preceding token not compatible with Ecmascript regex processor: https://regex101.com/r/7K1HfA/1

As for @MolbioUnige issue, it looks different as :arg1 doesn't match "/commitee". This is a Gherkin/Behat (I think) functionality to match greedy data and pass that to $arg1 variable.

When I do custom steps with variables, I write them like Behat contexts:

    /**
     * @Given /^(?:|I )am on "(?P<page>[^"]+)"$/
     */
    public function visit($page)
    {
        $this->visitPath($page);
    }

I'll check if ?P<var> can be stripped before being processed. Or maybe you see another lead @kieran-ryan?

Edit Here's an attempt to fix it, but I got other issues: https://github.com/thomas-hiron/language-service/commit/14fd2f7482f0f35b6c04b9ff689758b1bc7660c3 I'm investigating.

MolbioUnige commented 4 months ago

@thomas-hiron you can not add a test case in stepdefinition like that. For that, you'd need to change ExpressionBuilder.test.ts as well. My understanding is that this test file expects only two steps, with precise definition, the two that are defined actually.

Behat is the only php framework I know for Gherkin files. How do you test your feature files?

thomas-hiron commented 4 months ago

Yes, that was not meant to be merged, but to highlight the fact that ES regex processor doesn't handle Behat default regex! When I fixed the test with https://github.com/thomas-hiron/language-service/commit/14fd2f7482f0f35b6c04b9ff689758b1bc7660c3, the test was still failing due to another expected output as you mentioned!

I do test my feature files with Behat also!

For your precise issue, maybe the language service should replace :[a-z0-9]+ by the regex itself?

.replace(/:[a-z0-9]+/, ':[a-z0-9]+')

This should transform your :arg1 in an understandable regex for the language service?

MolbioUnige commented 4 months ago

See my PR

MolbioUnige commented 3 months ago

Dear maintainers, is there anything else I need to do for my PR to be taken in account?

develth commented 3 months ago

would be great that get this work or to provide another way to achieve that. Thanks!