Allow the named selector to skip the partial fallback by Amoifr · Pull Request #882 · minkphp/Mink

Amoifr · 2026-08-31T09:07:21Z

Fixes #880.

Implements the shape @aik099 settled on: an optional mode on ElementFinder deciding how the named selector resolves, and an optional mode on Session that forwards it. Nothing outside those two constructors changes, and both default to today's behaviour.

// exact matches only, the way "named_exact" behaves
$session = new Session($driver, $selectorsHandler, ElementFinder::NAMED_EXACT);

ElementFinder stays untouched from the outside, which is the point of passing a mode rather than a finder instance: users never name a class that is marked @internal.

On the tests you asked for

The mode reaching ElementFinder is checked through behaviour rather than through a getter, since a fallback costs an extra driver query and that is observable. SessionTest covers both directions:

testNamedModeIsForwardedToTheElementFinder builds a Session with NAMED_EXACT and asserts the driver is queried once, so no partial lookup happened.
testTheDefaultNamedModeStillFallsBackToPartial builds one without the argument and asserts the driver is queried twice.

ElementFinderTest covers each mode symmetrically, two tests per mode: testNamedFound and testNamedPartialFallback for the default, testNamedExactModeStillReturnsExactMatches and testNamedExactModeDoesNotFallBackToPartial for the strict one. I checked they fail against an unmodified src/.

One design note

The condition reads self::NAMED_EXACT !== $this->namedMode rather than testing for the default. It is the safer way round: a mode that is neither constant falls back to the historical behaviour instead of silently switching to strict.

I first added a runtime guard rejecting unknown modes, then dropped it. PHPStan flagged the test for it, because the @param self::NAMED_* annotation makes an invalid argument impossible statically, and this repository keeps level 8 clean without a single inline @phpstan-ignore. Inverting the condition made the guard unnecessary rather than merely unenforced.

Full suite green, PHPStan clean. Point 3 of your plan shipped separately in #881.

codecov · 2026-08-31T09:08:13Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.57%. Comparing base (9b08f62) to head (00bd0a5).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master     #882   +/-   ##
=========================================
  Coverage     98.56%   98.57%           
- Complexity      389      392    +3     
=========================================
  Files            24       24           
  Lines           909      912    +3     
=========================================
+ Hits            896      899    +3     
  Misses           13       13

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

andriokha · 2026-08-31T10:49:26Z

Hi @Amoifr, thanks for working on this, I'm super keen for it to get in! I had a couple of points of feedback:

If ElementFinder is internal, then so are the constants (even public) exposed on it. If we're asking end users to pass the constants to the Session constructor, I think they should be on a non-internal class (Session in this case).
As we can't use enums in PHP 7.2 and we're using constants in their place, I'd suggest validating $namedMode and throwing an InvalidArgumentException if it's not valid.

stof · 2026-08-31T11:07:44Z

I think we should rather put those constants on a dedicated class than on Session (if we were able to use a true enum, we would also have to name it)

aik099 · 2026-08-31T11:14:08Z

+        $this->assertEquals(array(), $finder->findAll('named', 'test', 'parent_xpath'));
+    }
+
+    public function testNamedExactModeStillReturnsExactMatches()


Please:

rename this test into testNamedExactModeDoesNotFallBackToPartial

drop the testNamedExactModeDoesNotFallBackToPartial (declared above this one)

Most of test in this class assert search-behavior and return-behavior in the same method. No need to overcomplicate this by checking both things in separate test methods.

Done — the two tests are now a single testNamedExactModeDoesNotFallBackToPartial asserting the search behaviour and the return value together, like the rest of the class.

aik099 · 2026-08-31T11:16:19Z

-    public function __construct(DriverInterface $driver, ?SelectorsHandler $selectorsHandler = null)
+    /**
+     * @param ElementFinder::NAMED_* $namedMode How the "named" selector is resolved: an exact match
+     *                                         falling back to a partial one, or an exact match only.


Maybe indentation is wrong (missing space before falling word).

Fixed: the annotation is a single line now, so there is nothing left to misalign.

aik099 · 2026-08-31T11:19:19Z

+
+    public function testNamedModeIsForwardedToTheElementFinder()
+    {
+        $selectorsHandler = $this->getMockBuilder('Behat\\Mink\\Selector\\SelectorsHandler')->getMock();


Please use ::class with corresponding class import instead of specifying it's FQCN as string.

Done, SelectorsHandler::class with the import — and createMock() rather than the builder while I was at it.

aik099 · 2026-08-31T11:24:49Z

Agree with moving new class constants into a Session class and it would work fine in the Mink 2.x branch.

Amoifr · 2026-08-31T12:39:15Z

Thanks all three of you. Pushed everything that is not the open question.

@andriokha you are right, and it undercuts my own argument on the issue: I said @internal means users never name the class, then asked them to write ElementFinder::NAMED_EXACT. The constants have to live somewhere public. Where is now the only thing left, and @stof and @aik099 do not agree:

@stof: I think we should rather put those constants on a dedicated class than on Session
@aik099: Agree with moving new class constants into a Session class

Say which and I will move them in the next push. If it is a dedicated class it also needs a name, which is @stof's point about naming the enum we cannot write.

The validation is back, @andriokha, with one consequence worth stating rather than hiding. The narrow @param self::NAMED_* and a runtime guard cannot coexist here: PHPStan then rejects the very test that passes an invalid value, and this repository keeps level 8 clean without a single inline @phpstan-ignore. I picked the guard and widened the annotation to string, because the guard protects everyone at runtime while the annotation only helps callers who run a static analyser. Tell me if you would rather have it the other way and I will swap them.

@aik099, the three review points:

the @param continuation was one space short, exactly as you saw. Fixed.
::class with the import, instead of the FQCN string. I left the two in prepareSession() alone since they predate this pull request.
the two exact-mode tests are now one, under the name you asked for. I kept the empty-result case rather than the matching one: it is the only case where the two modes behave differently, since a successful exact lookup never falls back in either mode, and testNamedFound already covers that path. It asserts the single driver call and the returned value together, the way the rest of the class does.

Suite green at 526 tests, PHPStan clean.

stof · 2026-08-31T12:40:56Z

The narrow @param self::NAMED_* and a runtime guard cannot coexist here: PHPStan then rejects the very test that passes an invalid value, and this repository keeps level 8 clean without a single inline @phpstan-ignore. I picked the guard and widened the annotation to string, because the guard protects everyone at runtime while the annotation only helps callers who run a static analyser. Tell me if you would rather have it the other way and I will swap them.

Actually, we can have the precise type in phpdoc and the runtime guard by setting treatPhpDocTypesAsCertain: false in the phpstan config.

stof · 2026-08-31T12:42:41Z

I suggest going for the separate class to hold the constants.

Based on the packagist installation stats, I'm considering dropping support for PHP <8.1, which would then allow turning this separate class into an actual enum before releasing it.

Amoifr · 2026-08-31T12:49:01Z

Done, @stof. The constants now live in Behat\Mink\Selector\NamedSelectorMode, and neither ElementFinder nor Session declares any.

I put it in the Selector namespace rather than next to ElementFinder, because it sits beside the two things it chooses between, ExactNamedSelector and PartialNamedSelector. The name reads the same way once it becomes a real enum, NamedSelectorMode::EXACT, so the eventual conversion should be a rename of the keyword and nothing else. The class is final with a private constructor in the meantime, so nobody starts instantiating it.

use Behat\Mink\Selector\NamedSelectorMode;

$session = new Session($driver, $selectorsHandler, NamedSelectorMode::EXACT);

That also answers @andriokha's objection properly: nothing users have to name is marked @internal any more.

Suite green at 526 tests, PHPStan clean.

andriokha · 2026-08-31T13:07:47Z

+     *
+     * This is the historical behaviour, and stays the default.


Suggested change

*

* This is the historical behaviour, and stays the default.

I'd suggest avoiding describing historical behavior - code comments should describe how things work now imho. Source control can give historical information.

Applied — the note about the historical behaviour is gone, the constants only describe what they do now. Git remembers the rest.

andriokha · 2026-08-31T13:12:23Z


-    public function __construct(DriverInterface $driver, ?SelectorsHandler $selectorsHandler = null)
+    /**
+     * @param string $namedMode How the "named" selector is resolved: one of the


I think the advantage of switching off treatPhpDocTypesAsCertain is we can be more specific with the type here without upsetting phpstan.

Suggested change

* @param string $namedMode How the "named" selector is resolved: one of the

* @param NamedSelectorMode::* $namedMode How the "named" selector is resolved: one of the

Applied, and it corrected something I had got wrong earlier in this PR: the narrow annotation and the runtime guard do coexist fine. Both Session and ElementFinder are typed NamedSelectorMode::*.

Amoifr · 2026-08-31T15:54:24Z

Both applied, thanks @andriokha. The second one corrects something I got wrong earlier in this pull request.

I claimed the narrow @param and the runtime guard could not coexist here. They can. With your suggestion the annotation is NamedSelectorMode::* on both Session and ElementFinder, the guard stays, its test stays, and PHPStan reports no error. I checked that PHPStan is genuinely still looking rather than silently skipping the file: passing an int at that same call site is reported straight away, and removing it goes back to clean. So callers get the static narrowing and everyone gets the runtime check, instead of the trade I described.

The historical note is gone from the constant, you are right that git carries that.

One thing I have left as is, so you can tell me if you would rather have it otherwise: codecov/patch is at 87.5% because of the private constructor on NamedSelectorMode, which by design can never be called. It is what makes the class a faithful placeholder for the enum @stof has in mind, since an enum cannot be instantiated either. I would rather keep it and live with the number than drop it to make the check green, but it is a one-line change if you disagree.

Suite green at 526 tests, PHPStan clean.

andriokha · 2026-08-31T16:11:23Z

One thing I have left as is, so you can tell me if you would rather have it otherwise

Just for transparency, I'm a nobody :)

I would rather keep it and live with the number than drop it to make the check green

Dunno if the maintainers think it's worth it, but you can always do something like the following prior to attempting to instantiate the class:

$this->expectException(\Error::class);

aik099 · 2026-09-01T03:39:55Z

You can exclude that enum-candidate class from coverage checks by adding corresponding PHPUnit annotation/attribute in its declaration.

Amoifr · 2026-09-01T07:30:50Z

Done, thanks @aik099. @codeCoverageIgnore on the private constructor, as an annotation rather than an attribute since #[CodeCoverageIgnore] needs PHPUnit 10 and the constraint here is ^8.5.22 || ^9.5.11.

I put it on the constructor rather than on the class declaration. It is the same thing today, the constants have no executable line, but it keeps the exclusion to the one member that can genuinely never run if the class ever grows. Say the word if you would rather have it on the class.

That also settles @andriokha's expectException(\Error::class) idea: it would have worked, but asserting that PHP refuses a private constructor tests PHP, not Mink.

Suite green at 526 tests, PHPStan clean.

stof · 2026-09-01T15:49:21Z


-    public function __construct(DriverInterface $driver, SelectorsHandler $selectorsHandler, ?Manipulator $xpathManipulator = null)
+    /**
+     * @param string $namedMode How the "named" selector is resolved: one of the


Suggested change

* @param string $namedMode How the "named" selector is resolved: one of the

* @param NamedSelectorMode::* $namedSelectorMode

Applied, with one small change: the parameter is called $namedMode, not $namedSelectorMode, so I kept the existing name to avoid documenting a parameter that does not exist.

Worth knowing that the narrowed type made PHPStan fail on ElementFinderTest, where testUnknownNamedModeIsRejected deliberately passes invalid values to exercise the runtime guard. I put a targeted @phpstan-ignore argument.type on that single line with a reason. This project usually keeps its exclusions in phpstan.dist.neon, so tell me if you would rather have it there and I will move it.

I would actually rename the parameter. What is named is the selector, not the mode.

I put a targeted @phpstan-ignore argument.type on that single line with a reason.

A targeted comment is fine for this case, as this is a case where we intentionally pass invalid data.

Renamed, you're right — the mode is a property of the selector, not something named. $namedSelectorMode now, for the constructor parameter and the property on both ElementFinder and Session, with the test names following suit (testUnknownNamedSelectorModeIsRejected, testNamedSelectorModeIsForwardedToTheElementFinder, testTheDefaultNamedSelectorModeStillFallsBackToPartial).

And thanks for settling the ignore, I'll leave the targeted comment where it is.

stof · 2026-09-01T15:49:34Z

     */
    private $xpathManipulator;
+    /**
+     * @var string one of the NamedSelectorMode constants


Suggested change

* @var string one of the NamedSelectorMode constants

* @var NamedSelectorMode::*

Applied. Session::__construct() was already documented as NamedSelectorMode::*, so ElementFinder was simply the odd one out. I also dropped the "one of the NamedSelectorMode constants" prose from both, since the type now says it better than the sentence did.

What is named is the selector, not the mode, so the constructor parameter and the property become $namedSelectorMode on both ElementFinder and Session, and the tests follow the same naming.

aik099 requested changes Aug 31, 2026

View reviewed changes

Amoifr force-pushed the feat-880-named-selector-mode branch from a20a286 to 6a1cd24 Compare August 31, 2026 12:38

Amoifr force-pushed the feat-880-named-selector-mode branch from 6a1cd24 to 890036b Compare August 31, 2026 12:48

aik099 approved these changes Aug 31, 2026

View reviewed changes

andriokha reviewed Aug 31, 2026

View reviewed changes

Amoifr force-pushed the feat-880-named-selector-mode branch from 890036b to bb330a7 Compare August 31, 2026 15:54

Allow the named selector to skip the partial fallback

e8f8bb2

Amoifr force-pushed the feat-880-named-selector-mode branch from bb330a7 to e8f8bb2 Compare September 1, 2026 07:30

stof requested changes Sep 1, 2026

View reviewed changes

Narrow the named mode phpdoc to the NamedSelectorMode constants

399e187

Amoifr mentioned this pull request Sep 1, 2026

Keep the query string when comparing the current address #883

Open

Rename the named selector mode parameter

00bd0a5

What is named is the selector, not the mode, so the constructor parameter and the property become $namedSelectorMode on both ElementFinder and Session, and the tests follow the same naming.

	* @param string $namedMode How the "named" selector is resolved: one of the
	* @param NamedSelectorMode::* $namedMode How the "named" selector is resolved: one of the

	* @param string $namedMode How the "named" selector is resolved: one of the
	* @param NamedSelectorMode::* $namedSelectorMode

	* @var string one of the NamedSelectorMode constants
	* @var NamedSelectorMode::*

Uh oh!

Conversation

Amoifr commented Aug 31, 2026

On the tests you asked for

One design note

Uh oh!

codecov Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

andriokha commented Aug 31, 2026

Uh oh!

stof commented Aug 31, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aik099 commented Aug 31, 2026

Uh oh!

Amoifr commented Aug 31, 2026

Uh oh!

stof commented Aug 31, 2026

Uh oh!

stof commented Aug 31, 2026

Uh oh!

Amoifr commented Aug 31, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Amoifr commented Aug 31, 2026

Uh oh!

andriokha commented Aug 31, 2026

Uh oh!

aik099 commented Sep 1, 2026

Uh oh!

Amoifr commented Sep 1, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Aug 31, 2026 •

edited

Loading