Skip to content

Conversation

@CybotTM
Copy link
Contributor

@CybotTM CybotTM commented Jan 8, 2026

Summary

  • Fix grid table termination bug - Tables now correctly terminate when followed by non-blank content (text, directives, other tables)
  • Modernize test suite - PHPUnit 10+ compatibility and improved assertions
  • Code cleanup - Remove dead code, fix typos, improve type annotations

Changes

Bug Fix

Grid tables previously required a trailing blank line to terminate correctly. When text or other RST content immediately followed a table, the parser would either:

  • Continue trying to parse non-table content as table rows
  • Produce malformed output

Solution: Added isTableRowLine() helper that checks if the next line starts with | (after trimming whitespace). The termination logic now breaks when the next line is either empty OR not a valid table row.

Test Suite Modernization

  • Replaced non-repeatable #[DataProvider] attributes with combined provider (PHPUnit 10+ compatibility)
  • Replaced assert() with assertInstanceOf() for better failure messages
  • Added 6 new edge case tests:
    • Table followed by text without blank line
    • Table at end of file
    • Table followed by indented text
    • Consecutive tables without blank line
    • Table followed by directive

Code Cleanup

  • Fixed typo: isDefintionLineisDefinitionLine
  • Removed unused $headerRows variable
  • Removed orphaned gridTableFollowUpTextProvider data provider
  • Removed stale comment about "broken table cases"
  • Fixed PHPDoc type for $headers parameter (list<TableRow> instead of non-empty-list)
  • Removed obsolete PHPStan ignore pattern

Test Plan

  • All 19 tests pass (47 assertions)
  • PHPStan level max - no errors
  • Security audit passed (ReDoS, DoS, injection vectors analyzed)

CybotTM added 11 commits January 8, 2026 14:44
Tables now correctly terminate when followed by non-table content
without requiring a blank line separator.

- Add isTableRowLine() helper to detect valid table rows
- Table ends when separator is followed by non-table content
- Remove obsolete PHPStan ignore for unreachable statement
The variable was assigned but never read.
This data provider was never referenced by any test method.
Tests that grid tables are parsed correctly when they end at EOF
without a trailing blank line.
Tests that grid tables terminate correctly when followed by
whitespace-prefixed text that is not a table row.
PHPUnit 10+ does not support multiple DataProvider attributes on a
single test method. This creates a combined tableCreationProvider
that yields from all three individual providers.

Also adds missing return type docblocks to the row span and column
span providers for PHPStan compliance.
PHPUnit assertions provide better failure messages and are not
affected by assert() being disabled in php.ini.
Verifies that the table parser correctly terminates when another
table follows immediately without a blank line separator.
Verifies that the table parser correctly terminates when a
reStructuredText directive follows immediately after the table.
Renamed to isDefinitionLine (added missing 'i').
Removed outdated comment about broken table cases (tests pass).
Also fixed @param type for headers to list<TableRow> since it can
be empty.
@CybotTM CybotTM changed the title [BUGFIX] Handle grid table termination without trailing blank line fix: Grid table termination and test suite modernization Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant