diff --git a/cpp-generator/experimental/impl_header_parser.cpp b/cpp-generator/experimental/impl_header_parser.cpp index 0f98e13..4ff7b4a 100644 --- a/cpp-generator/experimental/impl_header_parser.cpp +++ b/cpp-generator/experimental/impl_header_parser.cpp @@ -77,6 +77,33 @@ struct UnsupportedSpelling { }; static QList g_unsupported; +// --------------------------------------------------------------------------- +// STRUCTURE the record scanner could not read. +// +// #127 closed the hole where an unknown TYPE was admitted as `any`. This is the +// same hole one level down: a line inside a `struct` body that the scanner +// could not read as a field was `continue`d, and the struct was published +// anyway — MINUS that field. A field list is not a detail of a record, it IS +// the record: the promise every other language binds to. Publishing a shorter +// one silently ships a contract that disagrees with the header, and nothing +// downstream can tell, because a contract with three fields and a contract with +// two are both perfectly well-formed. +// +// Same discipline as g_unsupported: collected here, withdrawn when the struct +// turns out never to reach the contract (an internal helper promises nothing), +// and a hard parse error otherwise. +struct UnreadableDecl { + QString record; // the struct it was found in — the withdrawal key + QString text; // the declaration as written, comments removed + QString hint; // what to write instead +}; +static QList g_unreadable; + +// Structs that were opened but published no field at all. Referenced by the API +// they are still a defect — the emitted contract names a `type` it never +// declares — but no single line is at fault, so they are reported separately. +static QStringList g_emptyRecords; + // Drop the qualifiers that are about how a value is PASSED rather than what it // is: cppTypeToLidl normalizes with this, and the diagnostics compare against it // so `const nlohmann::json&` and `nlohmann::json` are recognised as the same @@ -395,6 +422,148 @@ static TypeExpr cppTypeToLidl(const QString& raw, const QString& context = QStri return { TypeExpr::Primitive, "any", {} }; } +// Remove comments from the already-merged logical lines, honouring string and +// character literals and carrying block-comment state across lines. +// +// The record scanner used to strip with a bare `indexOf("//")`. That is right +// for `std::string name; // what it is` and WRONG for +// `std::string url = "http://x";`, which it truncates inside the literal — the +// declaration then no longer ends in ';', and the field vanished. Harmless +// enough while an unreadable line was merely skipped; now that it is a build +// error, the same truncation would reject valid code, so the strip has to know +// what a literal is. Block comments are removed for the same reason: a field +// annotated `std::string id; /* note */` did not end in ';' either. +static QStringList stripCommentsFrom(const QStringList& lines) +{ + QStringList out; + bool inBlock = false; + for (const QString& line : lines) { + QString kept; + bool inStr = false; + bool inChr = false; + for (int i = 0; i < line.size(); ++i) { + const QChar c = line[i]; + const QChar n = (i + 1 < line.size()) ? line[i + 1] : QChar(); + if (inBlock) { + if (c == '*' && n == '/') { inBlock = false; ++i; } + continue; + } + if (inStr || inChr) { + kept += c; + if (c == '\\' && i + 1 < line.size()) { kept += n; ++i; } + else if (inStr && c == '"') inStr = false; + else if (inChr && c == '\'') inChr = false; + continue; + } + if (c == '/' && n == '*') { inBlock = true; ++i; continue; } + if (c == '/' && n == '/') break; + if (c == '"') inStr = true; + else if (c == '\'') inChr = true; + kept += c; + } + out.append(kept.trimmed()); + } + return out; +} + +// A `struct` DEFINITION opening, in the forms C++ is actually written in: +// +// struct Name { K&R — the only form the scanner used to accept +// +// struct Name Allman — the opening brace on the next line +// { +// +// plus the base-clause spelling of either. `struct Name;` is a forward +// declaration and stays out: there is no body to read. +// +// Allman was not a harmless stylistic omission. The struct was not a record AT +// ALL, so every mention of it in a signature fell through to the `any` fallback +// — which since #127 is a hard error whose hint tells the author to "declare a +// struct", the very thing they did declare. Where a brace sits cannot decide +// what a header means. +struct StructOpen { + QString name; + QString text; // the opening as written, for diagnostics + bool hasBase = false; + bool bodyOnOpeningLine = false; // `struct P { int64_t a; };` all on one line + int bodyStart = -1; // index of the first line INSIDE the body +}; + +static bool matchStructOpen(const QStringList& code, int i, StructOpen& out) +{ + // `[^{;]` in the base clause keeps `struct Name;` and the brace itself out + // of the capture. + static const QRegularExpression kandrRe( + "^struct\\s+(\\w+)\\s*(:[^{;]*)?\\{(.*)$"); + static const QRegularExpression headRe("^struct\\s+(\\w+)\\s*(:[^{;]*)?$"); + + const QRegularExpressionMatch km = kandrRe.match(code.at(i)); + if (km.hasMatch()) { + out.name = km.captured(1); + out.text = code.at(i); + out.hasBase = !km.captured(2).trimmed().isEmpty(); + out.bodyOnOpeningLine = !km.captured(3).trimmed().isEmpty(); + out.bodyStart = i + 1; + return true; + } + + const QRegularExpressionMatch hm = headRe.match(code.at(i)); + if (!hm.hasMatch()) return false; + // Allman: the next line carrying any code at all has to open the body. + // Anything else and this was not a definition (a `struct Name` mentioned in + // some other construct), so it is left alone exactly as before. + for (int j = i + 1; j < code.size(); ++j) { + if (code.at(j).isEmpty()) continue; + if (!code.at(j).startsWith('{')) return false; + out.name = hm.captured(1); + out.text = code.at(i); + out.hasBase = !hm.captured(2).trimmed().isEmpty(); + out.bodyOnOpeningLine = !code.at(j).mid(1).trimmed().isEmpty(); + out.bodyStart = j + 1; + return true; + } + return false; +} + +static void reportUnreadable(const QString& record, const QString& text, + const QString& hint) +{ + g_unreadable.append({ record, text.trimmed(), hint }); +} + +// Net brace depth a line adds, ignoring braces inside string and character +// literals: `std::string s = "{";` is balanced code even though it is not +// balanced text, and a body scan that believed the text would never find the +// end of the struct. +static int braceDelta(const QString& line) +{ + int delta = 0; + bool inStr = false; + bool inChr = false; + for (int i = 0; i < line.size(); ++i) { + const QChar c = line[i]; + if (inStr || inChr) { + if (c == '\\') { ++i; continue; } + if (inStr && c == '"') inStr = false; + else if (inChr && c == '\'') inChr = false; + continue; + } + if (c == '"') inStr = true; + else if (c == '\'') inChr = true; + else if (c == '{') ++delta; + else if (c == '}') --delta; + } + return delta; +} + +// What a contract field looks like. Named once because three diagnostics quote +// it, and a hint that describes a different grammar than the one enforced is +// worse than no hint. +static const QString kFieldFormHint = QStringLiteral( + "A contract field is ONE declaration per line, ending in `;` — `Type name;`, " + "optionally with a default (`= v` or `{v}`). A declaration wrapped across " + "several lines is joined for you; two declarations sharing one line are not."); + // Find `struct Name { Type field; ... };` blocks and turn them into `type` // declarations. // @@ -403,58 +572,171 @@ static TypeExpr cppTypeToLidl(const QString& raw, const QString& context = QStri // hand-written .lidl. Worse, a method mentioning the struct still parsed: its // type fell through to the opaque `any`, so the contract silently disagreed // with the header. +// +// Two things beyond that are new here, and they are the same idea from opposite +// ends. The body is read as DECLARATIONS rather than lines — physical lines are +// joined until the `;`, exactly as the caller already joins a method signature +// until its parentheses balance — so a wrapped field is the field the author +// wrote rather than nothing at all. And whatever is left over after that, and +// after the constructs that definitively are NOT fields, is reported instead of +// skipped: the scanner may not quietly decide that a line it cannot read was +// not worth publishing. static std::vector scanForRecords(const QStringList& lines) { - static QRegularExpression openRe("^struct\\s+(\\w+)\\s*\\{\\s*$"); - static QRegularExpression fieldRe("^([\\w:<>,\\s\\*]+?)\\s+(\\w+)\\s*(=[^;]*)?;$"); + // `\{[^;]*\}` accepts a brace initialiser beside the `=` form: a field + // written `std::string id{"none"};` is a field, and dropping it published a + // record whose defaults decided which members a consumer could see. + static QRegularExpression fieldRe( + "^([\\w:<>,\\s\\*]+?)\\s+(\\w+)\\s*(=[^;]*|\\{[^;]*\\})?;$"); + static QRegularExpression accessRe("^(public|private|protected)\\s*:"); + // Declarations that are legitimately not fields, and are skipped by rule + // rather than by failing to match. A `static` data member is not part of + // the object's value and never reaches the wire. + static QRegularExpression notAFieldRe( + "^(using|typedef|friend|template|static_assert|static|constexpr|inline)\\b"); + static QRegularExpression nestedTypeRe("^(struct|class|union|enum)\\b"); + + // Comments come off ONCE, up front, so every rule below sees code. + const QStringList code = stripCommentsFrom(lines); // TWO passes. A record field may name another record (`Blob inner;` inside // Wrapper), and cppTypeToLidl only answers Named() for a name already in // g_recordNames — so every struct name has to be registered before any // field is typed. One pass silently typed such a field as `any`, and the // generated codec then tried to encode a Blob as a LogosMap. - for (int i = 0; i < lines.size(); ++i) { - QRegularExpressionMatch om = openRe.match(lines.at(i).trimmed()); - if (om.hasMatch()) - g_recordNames.insert(om.captured(1)); + for (int i = 0; i < code.size(); ++i) { + StructOpen so; + if (matchStructOpen(code, i, so)) + g_recordNames.insert(so.name); } std::vector out; - for (int i = 0; i < lines.size(); ++i) { - const QString line = lines.at(i).trimmed(); - QRegularExpressionMatch om = openRe.match(line); - if (!om.hasMatch()) continue; + for (int i = 0; i < code.size(); ++i) { + StructOpen so; + if (!matchStructOpen(code, i, so)) continue; TypeDecl td; - td.name = om.captured(1).toStdString(); + td.name = so.name.toStdString(); // Withdraw this struct's diagnostics if it turns out to declare no // fields at all — nothing is published, so nothing is misreported. const int diagMark = g_unsupported.size(); - for (int j = i + 1; j < lines.size(); ++j) { - const QString body = lines.at(j).trimmed(); - if (body.startsWith("};")) break; - if (body.isEmpty() || body.startsWith("//")) continue; - // Strip a trailing line comment before matching: a field written - // `std::string name; // what it is` does not end in ';' and was - // silently DROPPED, publishing a record with a partial field list — - // the worst kind of wrong, because it looks like a contract. - QString field = body; - const int comment = field.indexOf("//"); - if (comment >= 0) field = field.left(comment).trimmed(); - if (field.isEmpty()) continue; - QRegularExpressionMatch fm = fieldRe.match(field); - if (!fm.hasMatch()) continue; - FieldDecl fd; - fd.name = fm.captured(2).toStdString(); - const QString spelling = fm.captured(1).trimmed(); - fd.type = cppTypeToLidl( - spelling, - QString("type '%1': field '%2'").arg(om.captured(1), fm.captured(2)), - spelling, om.captured(1), /*nameEmitted=*/true); - td.fields.push_back(fd); + + if (so.hasBase) { + reportUnreadable( + so.name, so.text, + QString("`struct %1` has a base class, and this parser reads one " + "header as text — the inherited members are not in front " + "of it. Publishing the struct would drop exactly the " + "fields it cannot see. Declare the record without a base " + "and give it the inherited fields explicitly.") + .arg(so.name)); + } + + if (so.bodyOnOpeningLine) { + // The body shares the opening line, and the scan below starts on the + // NEXT one, so there is nothing for it to read. Say so instead of + // publishing an empty record. + reportUnreadable(so.name, so.text, + QString("The body shares the line with the opening " + "brace. Put each field on its own line. %1") + .arg(kFieldFormHint)); + } else { + // The body is read as DECLARATIONS, not lines: physical lines are + // joined until the declaration is whole, which is a `;` at the + // struct's own brace depth — or a `}` there, which is how a member + // function DEFINED inline ends. Depth is tracked because a member + // function's body, and a nested type's, are declarations of their + // own that a `;` inside them must not be mistaken for the end of. + QString acc; + int depth = 1; // inside the struct + for (int j = so.bodyStart; j >= 0 && j < code.size(); ++j) { + QString body = code.at(j); + // An access specifier may share the line with a declaration, as + // in the class-body parser. Strip it before anything else, or + // `public: std::string id;` reads as a field whose TYPE is + // `public: std::string`. + while (true) { + const QRegularExpressionMatch am = accessRe.match(body); + if (!am.hasMatch()) break; + body = body.mid(am.capturedEnd()).trimmed(); + } + if (body.isEmpty()) continue; + + const int delta = braceDelta(body); + if (depth + delta <= 0) { + // End of the struct. Anything still accumulating never + // became a whole declaration — report it rather than + // dropping it on the way out. + if (!acc.isEmpty()) + reportUnreadable(so.name, acc, kFieldFormHint); + break; + } + + acc = acc.isEmpty() ? body : acc + ' ' + body; + depth += delta; + // Not a whole declaration yet: a field wrapped across physical + // lines is still the same field, and a `;` inside an inline + // member-function body does not end the member. + if (depth != 1 || !(acc.endsWith(';') || acc.endsWith('}'))) + continue; + + const QString decl = acc; + acc.clear(); + + // The DECLARATOR is everything before the first `=` or `{`. A + // default value may legally contain parentheses + // (`std::string id = makeId();`), and only parentheses in the + // declarator make the line a member function. + qsizetype cut = decl.size(); + const qsizetype eq = decl.indexOf('='); + const qsizetype brace = decl.indexOf('{'); + if (eq >= 0) cut = qMin(cut, eq); + if (brace >= 0) cut = qMin(cut, brace); + if (decl.left(cut).contains('(')) + continue; // member function / constructor / destructor + if (notAFieldRe.match(decl).hasMatch()) + continue; + + if (nestedTypeRe.match(decl).hasMatch()) { + // A nested type is not a field — and the scanner used to + // walk straight into its body, folding the INNER type's + // members into this record's field list and stopping at the + // inner `};`, so the published record was made of another + // type's fields and missing all of its own. + reportUnreadable( + so.name, decl, + QString("A nested type is not a field, and its own " + "members were being folded into `%1`. Declare it " + "at namespace scope — it becomes a contract " + "`type` of its own — and give `%1` a field of " + "that type.") + .arg(so.name)); + continue; + } + + const QRegularExpressionMatch fm = fieldRe.match(decl); + if (!fm.hasMatch()) { + reportUnreadable(so.name, decl, kFieldFormHint); + continue; + } + FieldDecl fd; + fd.name = fm.captured(2).toStdString(); + const QString spelling = fm.captured(1).trimmed(); + fd.type = cppTypeToLidl( + spelling, + QString("type '%1': field '%2'").arg(so.name, fm.captured(2)), + spelling, so.name, /*nameEmitted=*/true); + td.fields.push_back(fd); + } + } + + if (!td.fields.empty()) { + out.push_back(td); + } else { + while (g_unsupported.size() > diagMark) g_unsupported.removeLast(); + if (!g_emptyRecords.contains(so.name)) + g_emptyRecords.append(so.name); } - if (!td.fields.empty()) out.push_back(td); - else while (g_unsupported.size() > diagMark) g_unsupported.removeLast(); } return out; } @@ -469,7 +751,14 @@ static std::vector scanForRecords(const QStringList& lines) // is allowed to do. A struct earns its place in the contract by appearing in a // method or event signature — transitively, since a published record's own // fields may name others. -static void keepOnlyReferencedRecords(ModuleDecl& module) +// +// Returns that referenced set. It is the withdrawal key for BOTH diagnostic +// channels: a struct the API never names promises nothing, so neither an +// unsupported field type nor a line the scanner could not read is a defect in +// it. The set — not the published types — is what a structural diagnostic is +// tested against, because the very failures being reported are the ones that +// keep a struct OUT of module.types. +static std::set keepOnlyReferencedRecords(ModuleDecl& module) { auto mention = [](const TypeExpr& te, std::set& out) { std::function walk = [&](const TypeExpr& t) { @@ -506,6 +795,7 @@ static void keepOnlyReferencedRecords(ModuleDecl& module) for (const TypeDecl& td : module.types) if (referenced.count(td.name)) kept.push_back(td); module.types = std::move(kept); + return referenced; } // --------------------------------------------------------------------------- @@ -640,12 +930,14 @@ ImplParseResult parseImplHeader(const QString& headerPath, { ImplParseResult result; - // All three file-statics are per-parse state: one process generates for more + // Every file-static above is per-parse state: one process generates for more // than one module. g_recordNames is cleared HERE as well as beside // scanForRecords, because a name left over from the previous module's header // would otherwise be visible while this one's metadata events are typed. g_unmappableSpellings.clear(); g_unsupported.clear(); + g_unreadable.clear(); + g_emptyRecords.clear(); g_recordNames.clear(); QJsonArray metadataEvents; @@ -995,7 +1287,66 @@ done: // Now that every signature is known, drop the structs the API never // mentions — a header's internal helpers must not become published // contract types. - keepOnlyReferencedRecords(result.module); + const std::set referenced = keepOnlyReferencedRecords(result.module); + + // STRUCTURE the scanner could not read is a BUILD ERROR, not a shorter + // record. + // + // Reported before the type diagnostics below because it is the more + // fundamental failure: when the scanner could not read a struct's body, the + // types it did manage to read there are not a trustworthy account of it + // either. Reported after keepOnlyReferencedRecords, and tested against the + // REFERENCED set rather than the published one, for the reason given on that + // function: a helper struct the API never mentions may be as unreadable as + // it likes, while a struct that failed to publish anything is exactly the + // case that has to be caught. + { + QStringList reports; + QSet seen; + for (const UnreadableDecl& u : g_unreadable) { + if (!referenced.count(u.record.toStdString())) + continue; // struct never reaches the contract + const QString line = + QString(" type '%1': `%2`\n could not be read as a field. %3") + .arg(u.record, u.text, u.hint); + if (seen.contains(line)) continue; + seen.insert(line); + reports << line; + } + // A struct the API NAMES that published no field at all. Nothing above + // need have fired — a body of nothing but member functions reads + // perfectly well and yields no record — and the emitted contract would + // then reference a `type` it never declares, which no reader of the + // .lidl can resolve and no backend can generate. + for (const QString& name : g_emptyRecords) { + if (!referenced.count(name.toStdString())) continue; + bool explained = false; + for (const UnreadableDecl& u : g_unreadable) + if (u.record == name) { explained = true; break; } + if (explained) continue; + reports << QString( + " type '%1' is named by this module's API but declares " + "no field this parser could read, so no `type %1` is " + "emitted and the contract would name a type it never " + "declares.\n %2") + .arg(name, kFieldFormHint); + } + if (!reports.isEmpty()) { + result.error = + headerPath + ": " + QString::number(reports.size()) + + (reports.size() == 1 ? " declaration in a struct this module " + "publishes could not be read.\n\n" + : " declarations in structs this module " + "publishes could not be read.\n\n") + + reports.join("\n\n") + + "\n\nA `struct` in this header becomes a contract `type`, and its " + "field list IS the promise consumers in every language bind to. " + "Each of these used to be skipped, and the record published " + "without it — a contract missing a field is as well-formed as one " + "that has it, so nothing downstream could tell.\n"; + return result; + } + } // A C++ spelling with no LIDL type is a BUILD ERROR, not a silent `any`. // diff --git a/tests/experimental/test_impl_header_parser.cpp b/tests/experimental/test_impl_header_parser.cpp index 63857f2..47f6695 100644 --- a/tests/experimental/test_impl_header_parser.cpp +++ b/tests/experimental/test_impl_header_parser.cpp @@ -930,3 +930,392 @@ TEST_F(ImplHeaderParserTest, ReservedHookSpellingsAreNotContractDefects) ASSERT_EQ(r.module.methods.size(), 1u); EXPECT_EQ(r.module.methods[0].name, "f"); } + +// --------------------------------------------------------------------------- +// Structure the record scanner could not read +// +// #127 made an unknown TYPE a build error. These cover the same hole one level +// down: STRUCTURE. A line inside a struct body the scanner could not read as a +// field used to be `continue`d and the record published without it, and a +// contract missing a field is exactly as well-formed as one that has it — so +// nothing downstream, in any language, could tell. +// --------------------------------------------------------------------------- + +// A field declaration wrapped across physical lines is the field the author +// wrote. It used to be ABSENT from the published record, with exit code 0. +TEST_F(ImplHeaderParserTest, WrappedFieldDeclarationIsStillOneField) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "struct SplitRecord {\n" + " std::string id;\n" + " std::vector\n" + " tags;\n" + " uint64_t n;\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " SplitRecord echoSplit(const SplitRecord& v);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + ASSERT_EQ(r.module.types.size(), 1u); + const TypeDecl& td = r.module.types[0]; + ASSERT_EQ(td.fields.size(), 3u) << "a wrapped field was dropped from the record"; + EXPECT_EQ(td.fields[0].name, "id"); + EXPECT_EQ(td.fields[1].name, "tags"); + EXPECT_EQ(td.fields[1].type.kind, TypeExpr::Array); + EXPECT_EQ(td.fields[2].name, "n"); +} + +// Where the opening brace sits cannot decide what a header means. Allman used +// to make the struct not a record AT ALL, so every mention of it fell through +// to `any` — and since #127 to an error telling the author to declare a struct, +// which is the thing they declared. +TEST_F(ImplHeaderParserTest, AllmanBraceStructIsARecord) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "struct AllmanRecord\n" + "{\n" + " std::string id;\n" + " uint64_t n;\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " AllmanRecord echoAllman(const AllmanRecord& v);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + ASSERT_EQ(r.module.types.size(), 1u); + EXPECT_EQ(r.module.types[0].name, "AllmanRecord"); + ASSERT_EQ(r.module.types[0].fields.size(), 2u); + ASSERT_EQ(r.module.methods.size(), 1u); + EXPECT_EQ(r.module.methods[0].returnType.kind, TypeExpr::Named); + EXPECT_EQ(r.module.methods[0].returnType.name, "AllmanRecord"); +} + +// The K&R form has to keep parsing exactly as it did — this is the form every +// record in the workspace is written in. +TEST_F(ImplHeaderParserTest, KandRStructStillParses) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "struct NormalRecord {\n" + " std::string id;\n" + " uint64_t n;\n" + " std::vector payload;\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " NormalRecord echoNormal(const NormalRecord& v);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + ASSERT_EQ(r.module.types.size(), 1u); + ASSERT_EQ(r.module.types[0].fields.size(), 3u); + EXPECT_EQ(r.module.types[0].fields[2].type.name, "bstr"); +} + +// A nested type is not a field — and the scanner used to walk into its body, +// publish the INNER type's members as the outer record's field list, and stop +// at the inner `};` so none of the outer's own fields were ever seen. +TEST_F(ImplHeaderParserTest, NestedTypeInARecordIsReported) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "struct Outer {\n" + " struct Inner {\n" + " int64_t a;\n" + " };\n" + " Inner inner;\n" + " std::string label;\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " Outer echoOuter(const Outer& v);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_TRUE(r.hasError()) << "the outer record was published made of the " + "inner type's fields"; + EXPECT_TRUE(r.error.contains("type 'Outer'")) << r.error.toStdString(); + EXPECT_TRUE(r.error.contains("namespace scope")) << r.error.toStdString(); +} + +// Two declarations sharing a line matched nothing, so the struct published no +// field, so no `type` was emitted at all — and the .lidl still named it in the +// signature. That contract references a type it never declares. +TEST_F(ImplHeaderParserTest, RecordThatPublishesNothingIsReported) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "struct Pair {\n" + " std::string first; std::string second;\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " Pair echoPair(const Pair& v);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_TRUE(r.hasError()) << "emitted a contract naming an undeclared type"; + EXPECT_TRUE(r.error.contains("type 'Pair'")) << r.error.toStdString(); + EXPECT_TRUE(r.error.contains("ONE declaration per line")) << r.error.toStdString(); +} + +// A body of nothing but member functions reads perfectly well and yields no +// record. No single line is at fault, so it is reported on its own terms. +TEST_F(ImplHeaderParserTest, RecordWithNoFieldsAtAllIsReported) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "struct Fieldless {\n" + " std::string toString() const;\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " Fieldless echo(const Fieldless& v);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_TRUE(r.hasError()); + EXPECT_TRUE(r.error.contains("type 'Fieldless'")) << r.error.toStdString(); + EXPECT_TRUE(r.error.contains("never declares")) << r.error.toStdString(); +} + +// A base class carries members this line-based parser cannot see, so publishing +// the struct would drop exactly the fields it cannot read. +TEST_F(ImplHeaderParserTest, RecordWithABaseClassIsReported) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "struct Base {\n" + " std::string id;\n" + "};\n" + "struct Derived : Base {\n" + " uint64_t n;\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " Derived echo(const Derived& v);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_TRUE(r.hasError()) << "published `Derived` without the base's fields"; + EXPECT_TRUE(r.error.contains("type 'Derived'")) << r.error.toStdString(); + EXPECT_TRUE(r.error.contains("base class")) << r.error.toStdString(); +} + +// Constructs that definitively are NOT fields are skipped by RULE, so a record +// that legitimately carries them still parses — the error is for what the +// scanner cannot read, not for everything that is not a field. +TEST_F(ImplHeaderParserTest, NonFieldMembersAreSkippedNotReported) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "struct Rich {\n" + " Rich() = default;\n" + " ~Rich();\n" + " std::string toString() const;\n" + " static const int64_t kMax = 5;\n" + " using Alias = std::string;\n" + " std::string id;\n" + " int64_t n;\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " Rich echo(const Rich& v);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + ASSERT_EQ(r.module.types.size(), 1u); + ASSERT_EQ(r.module.types[0].fields.size(), 2u); + EXPECT_EQ(r.module.types[0].fields[0].name, "id"); + EXPECT_EQ(r.module.types[0].fields[1].name, "n"); +} + +// A default value is part of the field, whichever way it is written, and may +// itself contain parentheses — only parentheses in the DECLARATOR make a line a +// member function. The brace form used to be dropped outright. +TEST_F(ImplHeaderParserTest, DefaultedFieldsAreStillFields) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "struct Defaulted {\n" + " std::string id{\"none\"};\n" + " int64_t n = 0;\n" + " std::string made = makeId();\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " Defaulted echo(const Defaulted& v);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + ASSERT_EQ(r.module.types.size(), 1u); + ASSERT_EQ(r.module.types[0].fields.size(), 3u); + EXPECT_EQ(r.module.types[0].fields[0].name, "id"); + EXPECT_EQ(r.module.types[0].fields[2].name, "made"); +} + +// Comments come off before anything else — but a `//` inside a string literal +// is not a comment. Truncating there ends the declaration before its `;`, which +// used to drop the field and would now reject valid code. +TEST_F(ImplHeaderParserTest, CommentStripKnowsWhatALiteralIs) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "struct Commented {\n" + " std::string url = \"http://example.invalid/x\";\n" + " std::string id; // what it is\n" + " int64_t n; /* and this one */\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " Commented echo(const Commented& v);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + ASSERT_EQ(r.module.types.size(), 1u); + ASSERT_EQ(r.module.types[0].fields.size(), 3u); + EXPECT_EQ(r.module.types[0].fields[0].name, "url"); + EXPECT_EQ(r.module.types[0].fields[1].name, "id"); + EXPECT_EQ(r.module.types[0].fields[2].name, "n"); +} + +// The withdrawal that makes the whole thing safe: a helper struct the API never +// names is not part of the contract, so structure the scanner could not read +// inside it promises nothing. Every real module in the workspace carries one of +// these (openmetrics `ModuleSource`, the package manager's `PendingAction`). +TEST_F(ImplHeaderParserTest, UnreadableHelperStructDoesNotFailTheBuild) +{ + const QString helper = + "struct Helper {\n" + " struct Nested {\n" + " int64_t a;\n" + " };\n" + " std::string first; std::string second;\n" + "};\n"; + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, helper + + "class ProbeImpl {\n" + "public:\n" + " int64_t f(int64_t n);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + + // ...and the same struct DOES fail the build once a method publishes it. + QTemporaryDir dir2; + ASSERT_TRUE(dir2.isValid()); + const QString hp2 = probeHeader(dir2, helper + + "class ProbeImpl {\n" + "public:\n" + " Helper f(const Helper& v);\n" + "};\n"); + QString e2; + QTextStream es2(&e2); + auto r2 = parseImplHeader(hp2, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", es2); + ASSERT_TRUE(r2.hasError()); + EXPECT_TRUE(r2.error.contains("type 'Helper'")) << r2.error.toStdString(); +} + +// A member function DEFINED inline is a declaration of its own. The body's +// braces, and any `;` inside it, belong to the member — not to the record — so +// the fields declared after it are still the record's fields. (An earlier cut of +// this change ended the scan at the body's closing brace and lost them.) +TEST_F(ImplHeaderParserTest, InlineMemberBodyDoesNotEndTheRecord) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "struct Inlined {\n" + " std::string id;\n" + " void reset() {\n" + " id = \"\";\n" + " }\n" + " std::string mark() const { return id; }\n" + " int64_t n;\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " Inlined echo(const Inlined& v);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + ASSERT_EQ(r.module.types.size(), 1u); + ASSERT_EQ(r.module.types[0].fields.size(), 2u) + << "an inline member-function body swallowed a field"; + EXPECT_EQ(r.module.types[0].fields[0].name, "id"); + EXPECT_EQ(r.module.types[0].fields[1].name, "n"); +} + +// A brace INITIALISER may wrap across lines too, and its braces must not read +// as a scope the scanner should skip. +TEST_F(ImplHeaderParserTest, WrappedBraceInitialiserIsStillOneField) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "struct Init {\n" + " std::vector tags{\n" + " \"a\", \"b\"};\n" + " int64_t n;\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " Init echo(const Init& v);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + ASSERT_EQ(r.module.types.size(), 1u); + ASSERT_EQ(r.module.types[0].fields.size(), 2u); + EXPECT_EQ(r.module.types[0].fields[0].name, "tags"); + EXPECT_EQ(r.module.types[0].fields[0].type.kind, TypeExpr::Array); + EXPECT_EQ(r.module.types[0].fields[1].name, "n"); +} + +// A brace inside a string literal is not a scope. Believing the text would make +// the scanner miss the end of the struct entirely. +TEST_F(ImplHeaderParserTest, BraceInAStringLiteralIsNotAScope) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "struct Literal {\n" + " std::string tmpl = \"{\";\n" + " int64_t n;\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " Literal echo(const Literal& v);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + ASSERT_EQ(r.module.types.size(), 1u); + ASSERT_EQ(r.module.types[0].fields.size(), 2u); + EXPECT_EQ(r.module.types[0].fields[1].name, "n"); +}