Skip to content

Commit 83e364a

Browse files
committed
fix: throw exception for conditional flow methods without underscore prefix on PropelConditionalProxy
Previously, __call() silently returned $this for ALL method calls, including conditional flow methods called without the underscore prefix (e.g. endif() instead of _endif()). This made such typos impossible to detect — the proxy would swallow the call and all subsequent chained calls, producing silently incorrect query results. Now detects calls to conditional flow methods (if, elseif, else, endif) without the underscore prefix and throws a BadMethodCallException with a helpful message. All other method calls are still silently skipped as before, since Criteria subclasses use __call for virtual methods (filterByX, orderByX, etc.).
1 parent 6259787 commit 83e364a

File tree

2 files changed

+78
-0
lines changed

2 files changed

+78
-0
lines changed

src/Propel/Runtime/Util/PropelConditionalProxy.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
namespace Propel\Runtime\Util;
1010

11+
use BadMethodCallException;
1112
use Propel\Runtime\ActiveQuery\Criteria;
1213

1314
/**
@@ -171,13 +172,31 @@ public function getCriteriaOrProxy()
171172
}
172173

173174
/**
175+
* Catches calls to non-conditional methods when the conditional state is false.
176+
*
177+
* Most method calls are silently skipped (the condition is false, so they
178+
* should not execute). However, calling a conditional flow method without
179+
* the underscore prefix (e.g. endif() instead of _endif()) is always a bug
180+
* that breaks the entire query chain — so we catch that specific mistake.
181+
*
174182
* @param string $name
175183
* @param array $arguments
176184
*
185+
* @throws \BadMethodCallException if the method name matches a conditional flow method without underscore prefix
186+
*
177187
* @return $this
178188
*/
179189
public function __call(string $name, array $arguments)
180190
{
191+
if (in_array($name, ['if', 'elseif', 'else', 'endif'], true)) {
192+
throw new BadMethodCallException(sprintf(
193+
'Call to undefined method %s::%s(). Did you mean "_%s" (with underscore prefix)?',
194+
get_class($this->criteria),
195+
$name,
196+
$name,
197+
));
198+
}
199+
181200
return $this;
182201
}
183202
}

tests/Propel/Tests/Runtime/Util/PropelConditionalProxyTest.php

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
namespace Propel\Tests\Runtime\Util;
1010

11+
use BadMethodCallException;
1112
use Propel\Runtime\ActiveQuery\Criteria;
1213
use Propel\Runtime\Util\PropelConditionalProxy;
1314
use Propel\Tests\Helpers\BaseTestCase;
@@ -53,6 +54,64 @@ public function testFluidInterface()
5354
$this->assertEquals($p->_else(), $criteria, '_else returns fluid interface');
5455
}
5556

57+
/**
58+
* @return void
59+
*/
60+
public function testValidMethodCallsAreSkippedSilently()
61+
{
62+
$criteria = new ProxyTestCriteria();
63+
$p = new TestPropelConditionalProxy($criteria, false);
64+
65+
// Methods that exist on the criteria should be silently skipped
66+
$this->assertSame($p, $p->test(), 'Valid method calls should return $this when condition is false');
67+
$this->assertSame($p, $p->dummy(), 'Valid method calls should return $this when condition is false');
68+
$this->assertFalse($criteria->getTest(), 'Method should not have been executed on criteria');
69+
}
70+
71+
/**
72+
* @return void
73+
*/
74+
public function testArbitraryMethodCallsAreSkippedSilently()
75+
{
76+
$criteria = new ProxyTestCriteria();
77+
$p = new TestPropelConditionalProxy($criteria, false);
78+
79+
// Arbitrary method names should be silently skipped, since Criteria
80+
// subclasses use __call for virtual methods (filterByX, orderByX, etc.)
81+
$this->assertSame($p, $p->filterBySomething('value'));
82+
$this->assertSame($p, $p->orderByName());
83+
$this->assertSame($p, $p->anyOtherMethod());
84+
}
85+
86+
/**
87+
* @dataProvider conditionalFlowMethodsWithoutUnderscoreProvider
88+
*
89+
* @return void
90+
*/
91+
public function testConditionalFlowMethodsWithoutUnderscoreThrowException(string $method)
92+
{
93+
$criteria = new ProxyTestCriteria();
94+
$p = new TestPropelConditionalProxy($criteria, false);
95+
96+
$this->expectException(BadMethodCallException::class);
97+
$this->expectExceptionMessage(sprintf('Did you mean "_%s"', $method));
98+
99+
$p->$method();
100+
}
101+
102+
/**
103+
* @return array<array<string>>
104+
*/
105+
public static function conditionalFlowMethodsWithoutUnderscoreProvider(): array
106+
{
107+
return [
108+
['if'],
109+
['elseif'],
110+
['else'],
111+
['endif'],
112+
];
113+
}
114+
56115
/**
57116
* @return void
58117
*/

0 commit comments

Comments
 (0)