From bb6d87b6ec7a64814df48fbdb10cf5290ce4b724 Mon Sep 17 00:00:00 2001 From: Dario Lipicar Date: Tue, 9 Jun 2026 14:46:15 -0300 Subject: [PATCH] fix: parse declarations on same line as `logos_events:` / access specifier (#76) (#81) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: parse declarations on same line as logos_events/access specifier (#76) The impl header parser updated its section state and immediately broke out of line processing when it matched `logos_events:` (or `public:`/`private:`), discarding any declaration on the same physical line. This meant clang-format / prettier output like logos_events : void versionReady(const std::string &version); silently dropped the event, while the newline-separated form parsed fine — the same valid C++ was handled differently based on formatting. Strip any leading section specifiers in a loop, updating the section state, then let the remainder of the line fall through to the declaration parser. Brace counting still happens once per physical line and blank-line doc-comment reset is preserved. Adds a regression test (SameLineSectionSpecifiers) with a fixture covering the exact prettier form from the issue, a follow-on same-line event, the newline form alongside it, and the symmetric inline `public:` method case. Fixes #76 Co-Authored-By: Claude Opus 4.8 (1M context) * fix: attach doc comments to same-line logos_events/access-specifier decls Address review feedback: the first pass cleared pendingDoc on every specifier match, so a `///` comment above a collapsed `logos_events : void foo();` did not attach to the event. In the collapsed form there is nowhere else to put the doc comment, so this left documentation formatting-dependent — the same bug class as #76, one level up. Only clear pendingDoc for a *bare* specifier (a section boundary, matching Qt `signals:` semantics); when a declaration shares the line, keep the pending doc so the declaration parser attaches it. Extend the fixture with a `///`-documented same-line event and assert the description is captured. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../experimental/impl_header_parser.cpp | 42 ++++++++--- .../fixtures/same_line_events_impl.h | 31 ++++++++ .../fixtures/same_line_events_metadata.json | 7 ++ .../experimental/test_impl_header_parser.cpp | 71 +++++++++++++++++++ 4 files changed, 142 insertions(+), 9 deletions(-) create mode 100644 tests/experimental/fixtures/same_line_events_impl.h create mode 100644 tests/experimental/fixtures/same_line_events_metadata.json diff --git a/cpp-generator/experimental/impl_header_parser.cpp b/cpp-generator/experimental/impl_header_parser.cpp index 4e5d618..ead1556 100644 --- a/cpp-generator/experimental/impl_header_parser.cpp +++ b/cpp-generator/experimental/impl_header_parser.cpp @@ -322,27 +322,51 @@ ImplParseResult parseImplHeader(const QString& headerPath, } } + // A section specifier may be followed by a declaration on the + // *same* physical line — e.g. clang-format / prettier collapse + // logos_events: + // void versionReady(const std::string& version); + // into `logos_events : void versionReady(const std::string& version);`. + // Strip any leading specifiers, updating the section state, and + // let whatever remains fall through to the declaration parser + // below — otherwise everything after the colon is discarded and + // the same valid C++ is parsed differently based on formatting. + // // `logos_events:` takes precedence over the standard access // specifiers: it's a separate section that the codegen pulls // event prototypes from. (At preprocess time, `logos_events` // expands to `public`, but the raw source still carries the // token we recognise here.) - if (eventsRe.match(line).hasMatch()) { - state = InLogosEvents; - pendingDoc.clear(); - break; - } - - { + bool specifierStripped = false; + while (true) { + QRegularExpressionMatch em = eventsRe.match(line); + if (em.hasMatch()) { + state = InLogosEvents; + line = line.mid(em.capturedEnd()).trimmed(); + specifierStripped = true; + continue; + } QRegularExpressionMatch am = accessRe.match(line); if (am.hasMatch()) { QString spec = am.captured(1); if (spec == "public") state = InPublic; else state = InPrivate; - pendingDoc.clear(); - break; + line = line.mid(am.capturedEnd()).trimmed(); + specifierStripped = true; + continue; } + break; } + // A *bare* specifier (nothing after the colon) is a section + // boundary and resets any pending doc-comment, mirroring Qt's + // `signals:`. But when a declaration shares the line, the doc + // comment preceding the whole line must still attach to that + // declaration — otherwise documentation, like the declaration + // itself (#76), would become formatting-dependent. So only clear + // here for the bare form; the same-line form keeps pendingDoc and + // attaches it in the declaration parser below. + if (specifierStripped && line.isEmpty()) + pendingDoc.clear(); // Only doc comments (/// or /** ... */ / /*! ... */) accumulate as // the pending description for the next method. Plain // and /* diff --git a/tests/experimental/fixtures/same_line_events_impl.h b/tests/experimental/fixtures/same_line_events_impl.h new file mode 100644 index 0000000..e0e46ee --- /dev/null +++ b/tests/experimental/fixtures/same_line_events_impl.h @@ -0,0 +1,31 @@ +#pragma once +#include +#include + +// Parser tests only read this as text. Regression fixture for issue #76: +// a section specifier and a declaration collapsed onto one *physical* line +// (as clang-format / prettier produce) must be parsed exactly like the +// newline-separated form. The code after the colon must NOT be discarded, +// otherwise the same valid C++ is processed differently based on formatting. +class SameLineEventsImpl { +public: + SameLineEventsImpl() = default; + + // Access specifier + declaration on one line: the method is still found. + public: std::string greet(const std::string& name); + + // The exact prettier-formatted form from the issue: `logos_events`, + // a space, the colon, then the event prototype — all on one line. The + // `///` doc comment above must still attach to the event: in the + // collapsed form there is nowhere else to put it, so dropping it would + // make documentation formatting-dependent too. + /// Fired once the latest version is known. + logos_events : void versionReady(const std::string &version); + + // A further event after the section is already open (still same-line). + void downloadProgress(const std::string &id, int64_t percent); + + // The newline-separated form keeps working alongside the collapsed form. +logos_events: + void shutdown(); +}; diff --git a/tests/experimental/fixtures/same_line_events_metadata.json b/tests/experimental/fixtures/same_line_events_metadata.json new file mode 100644 index 0000000..9f57cb3 --- /dev/null +++ b/tests/experimental/fixtures/same_line_events_metadata.json @@ -0,0 +1,7 @@ +{ + "name": "same_line_events_mod", + "version": "1.0.0", + "description": "Fixture for same-line section specifiers (issue #76)", + "category": "test", + "dependencies": [] +} diff --git a/tests/experimental/test_impl_header_parser.cpp b/tests/experimental/test_impl_header_parser.cpp index 167b4c0..7892aab 100644 --- a/tests/experimental/test_impl_header_parser.cpp +++ b/tests/experimental/test_impl_header_parser.cpp @@ -349,3 +349,74 @@ TEST_F(ImplHeaderParserTest, EventDocCommentsFromHeader) EXPECT_EQ(r.module.events[2].name, "shutdown"); EXPECT_EQ(r.module.events[2].description, "Single-line documented event."); } + +// --------------------------------------------------------------------------- +// Issue #76: a section specifier and a declaration on the *same* physical +// line (`logos_events : void foo();`, as clang-format / prettier produce) +// must be parsed identically to the newline-separated form. The code after +// the colon must not be discarded. +// --------------------------------------------------------------------------- + +TEST_F(ImplHeaderParserTest, SameLineSectionSpecifiers) +{ + auto r = parseImplHeader( + fixturesDir() + "/same_line_events_impl.h", + "SameLineEventsImpl", + fixturesDir() + "/same_line_events_metadata.json", + err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + + auto findEvent = [&](const QString& name) -> const EventDecl* { + for (const auto& e : r.module.events) + if (e.name == name) return &e; + return nullptr; + }; + auto findMethod = [&](const QString& name) -> const MethodDecl* { + for (const auto& m : r.module.methods) + if (m.name == name) return &m; + return nullptr; + }; + + // The exact prettier form from the issue: + // logos_events : void versionReady(const std::string &version); + // Previously the prototype after the colon was discarded entirely. + const EventDecl* versionReady = findEvent("versionReady"); + ASSERT_NE(versionReady, nullptr) + << "Same-line `logos_events :` prototype must still be parsed"; + ASSERT_EQ(versionReady->params.size(), 1); + EXPECT_EQ(versionReady->params[0].name, "version"); + EXPECT_EQ(versionReady->params[0].type.name, "tstr"); + // The `///` doc comment above the collapsed line must attach: in the + // same-line form there is nowhere else for it to go, so documentation + // must not be formatting-dependent either. + EXPECT_EQ(versionReady->description, "Fired once the latest version is known."); + + // An event declared after the section is already open, also same-line. + const EventDecl* downloadProgress = findEvent("downloadProgress"); + ASSERT_NE(downloadProgress, nullptr); + ASSERT_EQ(downloadProgress->params.size(), 2); + EXPECT_EQ(downloadProgress->params[0].name, "id"); + EXPECT_EQ(downloadProgress->params[0].type.name, "tstr"); + EXPECT_EQ(downloadProgress->params[1].name, "percent"); + EXPECT_EQ(downloadProgress->params[1].type.name, "int"); + + // The newline-separated form keeps working alongside the collapsed form. + EXPECT_NE(findEvent("shutdown"), nullptr); + + // Exactly the three events above — no phantom or dropped entries. + EXPECT_EQ(r.module.events.size(), 3); + + // The symmetric case: `public : ` on one line must surface the + // method too (the access specifier no longer swallows the declaration). + const MethodDecl* greet = findMethod("greet"); + ASSERT_NE(greet, nullptr) + << "Same-line `public:` declaration must still be parsed"; + EXPECT_EQ(greet->returnType.name, "tstr"); + ASSERT_EQ(greet->params.size(), 1); + EXPECT_EQ(greet->params[0].name, "name"); + EXPECT_EQ(greet->params[0].type.name, "tstr"); + + // The same-line events must land in events[], never leak into methods[]. + EXPECT_EQ(findMethod("versionReady"), nullptr); + EXPECT_EQ(findMethod("downloadProgress"), nullptr); +}