Skip to content

Fix padding bug #48

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
donatj merged 5 commits into from
Apr 17, 2026
Merged

Fix padding bug #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9f587c8
Fix padding bug
donatj Apr 17, 2026
a74acdf
Add eof mid padding parse for improved coverage
donatj Apr 17, 2026
a84618e
Improve simple position test
donatj Apr 17, 2026
3d0fe5d
Update test/Integration/ParserTest.php
donatj Apr 17, 2026
93a467e
Fix EOF-after-width padWidth bug: only rewind when next token is a digit
Copilot Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions src/Parser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ private function lexSprintf( Emitter $emitter, StringLexer $lexer ) : void {
$lexer->next();
$next = $lexer->next();
$arg = $int;
} else {
$padWidth = $int;
$next = $lexer->next();
}
}

Expand Down Expand Up @@ -133,14 +136,10 @@ private function lexSprintf( Emitter $emitter, StringLexer $lexer ) : void {
break;
}

if( $padChar !== null ) {
if( ctype_digit($next->getString()) ) {
$lexer->rewind();
$peek = $lexer->peek();
if( ctype_digit($peek->getString()) ) {
$padWidth = $this->eatInt($lexer);
}

$next = $lexer->next();
$padWidth = $this->eatInt($lexer);
$next = $lexer->next();
}

if( $next->getString() === '.' ) {
Expand Down
58 changes: 47 additions & 11 deletions test/Integration/ParserTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,20 @@
class ParserTest extends TestCase {

/**
* @param bool|string $valid
* @dataProvider parseStringProvider
*/
public function testParsing( string $input, string $serialized ) : void {
$emitter = new class implements Emitter {
public function testParsing( string $input, string $serialized, $valid ) : void {
$lexemeEmitter = new LexemeEmitter;
$emitter = new class($lexemeEmitter) implements Emitter {

public Emitter $lexemeEmitter;
public string $serialized = '';
public function __construct( Emitter $emitter ) {
$this->lexemeEmitter = $emitter; }
Comment thread
donatj marked this conversation as resolved.

public function emit( Lexeme $lexItem ) : void {
$this->lexemeEmitter->emit($lexItem);
if( $lexItem instanceof ArgumentLexeme ) {
$this->serialized .= "[{$lexItem->getLexItemType()}={$lexItem->getVal()}:{$lexItem->getPos()}||{$lexItem->getArg()}|pos:{$lexItem->getShowPositive()}|{$lexItem->getPadChar()}|{$lexItem->getPadWidth()}|left:{$lexItem->getLeftJustified()}|{$lexItem->getPrecision()}]";
} else {
Expand All @@ -31,38 +37,68 @@ public function emit( Lexeme $lexItem ) : void {

(new Parser($emitter))->parseStr($input);
$this->assertSame($serialized, $emitter->serialized);

$invalid = $lexemeEmitter->getLexemes()->getInvalid();
$this->assertSame(
($invalid === null),
!($valid === false),
sprintf(
Comment on lines +42 to +45

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$valid is documented as bool|string and the provider supplies canonical strings in a couple cases, but the test only treats it as a boolean validity flag ($valid === false). This makes the data provider contract misleading and leaves the canonical string values unused.

Either assert the canonical string when $valid is a string (if canonicalization is a requirement), or change the provider/doc/type expectations to a simple boolean validity indicator.

Copilot uses AI. Check for mistakes.
'Expected validity: %s, but got %s. Invalid lexeme: %s',
($valid === false) ? 'invalid' : 'valid',
($invalid === null) ? 'valid' : 'invalid',
($invalid !== null) ? sprintf("'%s' at position %d", $invalid->getVal(), $invalid->getPos()) : 'none'
));
}

/**
* @return array<array{string,string}>
* @return array<array{string,string,bool|string}> the string to test, its summarized parse, canonical form/true if already canonical/false if invalid
*/
public static function parseStringProvider() : array {
return [

[ 'What %%%f percent', '[!=What :0][!=%:6][f=f:8|||pos:|||left:|][!= percent:9]' ],
[ '%% foo %%', '[!=%:1][!= foo :2][!=%:8]', true ],
[ '%11d%+22d%-33d', '[d=11d:1|||pos:||11|left:|][d=+22d:5|||pos:1||22|left:|][d=-33d:10|||pos:||33|left:1|]', true ],
[ 'What %%%f percent', '[!=What :0][!=%:6][f=f:8|||pos:|||left:|][!= percent:9]', true ],
// test all padding types
[ '%012d % 12d %\'x12d', '[d=012d:1|||pos:|0|12|left:|][!= :5][d= 12d:7|||pos:| |12|left:|][!= :11][d=\'x12d:13|||pos:|x|12|left:|]' ],
[ 'foo%sbar', '[!=foo:0][s=s:4|||pos:|||left:|][!=bar:5]' ],
[ 'f%1$\'x-10d soup', '[!=f:0][d=1$\'x-10d:2||1|pos:|x|10|left:1|][!= soup:10]' ],
[ '%012d % 12d %\'x12d', '[d=012d:1|||pos:|0|12|left:|][!= :5][d= 12d:7|||pos:| |12|left:|][!= :11][d=\'x12d:13|||pos:|x|12|left:|]', true ],
[ 'foo%sbar', '[!=foo:0][s=s:4|||pos:|||left:|][!=bar:5]', true ],
[ 'f%1$\'x-10d soup', '[!=f:0][d=1$\'x-10d:2||1|pos:|x|10|left:1|][!= soup:10]', true ],
[
'%s %d foo %15$\'c10.2c SOUP',
'[s=s:1|||pos:|||left:|][!= :2][d=d:4|||pos:|||left:|][!= foo :5][c=15$\'c10.2c:11||15|pos:|c|10|left:|2][!= SOUP:21]',
true,
],
[
'this %% is my %s to %1$\'x10d parse %2$s longer string %15$s',
'[!=this :0][!=%:6][!= is my :7][s=s:15|||pos:|||left:|][!= to :16][d=1$\'x10d:21||1|pos:|x|10|left:|][!= parse :28][s=2$s:36||2|pos:|||left:|][!= longer string :39][s=15$s:55||15|pos:|||left:|]',
true,
],

'test positional arguments' => [
'%g %2$s-%1$f %u',
'[g=g:1|||pos:|||left:|][!= :2][s=2$s:4||2|pos:|||left:|][!=-:7][f=1$f:9||1|pos:|||left:|][!= :12][u=u:14|||pos:|||left:|]',
true,
],

'invalid string handling' => [ '100%', '[!=100:0][=:4|||pos:|||left:|]' ],
'invalid string handling' => [
'100%',
'[!=100:0][=:4|||pos:|||left:|]',
false,
],

'test positive argument' => [ '%+10d %-+2d %+-2d', '[=+1:1|||pos:1|||left:|][!=0d :3][=-+2:7|||pos:1|||left:1|][!=d :10][=+-2:13|||pos:1|||left:1|][!=d:16]' ],
'eof mid padding parse' => [ '%+10', '[=+10:1|||pos:1||10|left:|]', false],
'eof mid padding parse no flags' => [ '%10', '[=10:1|||pos:||10|left:|]', false],

'handle dumb flag parsing' => [ "%---+++---+-'x10d", '[d=---+++---+-\'x10d:1|||pos:1|x|10|left:1|]' ],
'test positive argument' => [
'%+10d %-+2d %+-2d',
'[d=+10d:1|||pos:1||10|left:|][!= :5][d=-+2d:7|||pos:1||2|left:1|][!= :11][d=+-2d:13|||pos:1||2|left:1|]',
'%+10d %-+2d %-+2d',
],

'handle dumb flag parsing' => [
"%---+++---+-'x10d",
'[d=---+++---+-\'x10d:1|||pos:1|x|10|left:1|]',
"%'x-+10d",
],
];
}

Expand Down