diff --git a/cpp-generator/experimental/impl_header_parser.cpp b/cpp-generator/experimental/impl_header_parser.cpp index c4de127..0f98e13 100644 --- a/cpp-generator/experimental/impl_header_parser.cpp +++ b/cpp-generator/experimental/impl_header_parser.cpp @@ -51,13 +51,151 @@ static QSet g_recordNames; // (same reason g_recordNames is file-static); parseImplHeader drains it. static QStringList g_unmappableSpellings; -static TypeExpr cppTypeToLidl(const QString& raw) +// --------------------------------------------------------------------------- +// Spellings with NO LIDL type at all. +// +// These used to reach the `any` fallback at the bottom of cppTypeToLidl, and +// `any` is ADMITTED by every backend gate — so the declaration was accepted, the +// published contract said `any`, and the generated dispatch handed the raw +// nlohmann::json straight to the author's parameter. That either worked by luck +// through nlohmann's implicit conversions, threw at call time, or emitted a +// non-canonical wire value. Nothing said a word. +// +// Now every one of them is recorded here and parseImplHeader turns the list into +// a hard parse error naming the offending C++ type and the fix. +// +// cppTypeToLidl still RETURNS the historical `any` for these: the diagnostic and +// the mapping are separate, so a declaration whose diagnostic is later discarded +// (a helper struct that never reaches the contract, a reserved lifecycle hook) +// produces byte-identical output to before. +struct UnsupportedSpelling { + QString context; // "method 'foo': parameter 'bar'" + QString record; // non-empty when this came from a record field + QString declared; // the full spelling as written on the declaration + QString offending; // the spelling that has no LIDL type (may be nested) + QString hint; // what to write instead +}; +static QList g_unsupported; + +// 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 +// spelling instead of reading as a type nested inside itself. +static QString normalizeCppSpelling(const QString& raw) { - // Normalize: strip const, &, leading/trailing whitespace QString t = raw.trimmed(); t.remove(QRegularExpression("^const\\s+")); t.remove(QRegularExpression("\\s*&$")); - t = t.trimmed(); + return t.trimmed(); +} + +// Collapse whitespace so `unsigned long int` and `unsigned long int` are one +// key, and drop the redundant `int` from the multi-word integer spellings. +static QString normalizeNumericSpelling(QString t) +{ + t = t.simplified(); + static const QRegularExpression trailingInt("\\s+int$"); + if (t != "int" && t.contains(' ')) + t.remove(trailingInt); + return t; +} + +// What to write instead. Every hint names a spelling that IS in the contract, +// because "unsupported" without a replacement just moves the guesswork. +static QString unsupportedHint(const QString& t) +{ + static const QString kWidenNote = + "Widening is source-compatible for every caller; a narrow type on the " + "wire is not, which is why LIDL has none."; + + // uint8_t has exactly ONE meaning in this contract and it is not a number. + if (t == "uint8_t" || t == "std::uint8_t") + return "uint8_t means BYTES here, and only as `std::vector` " + "(LIDL `bstr`). For a small number declare `uint64_t` (LIDL " + "`uint`); for binary data declare `std::vector`."; + + const QString n = normalizeNumericSpelling(t); + static const QSet kUnsigned = { + "unsigned", "unsigned char", "unsigned short", "unsigned long", + "unsigned long long", "uint16_t", "uint32_t", "size_t", + "uintptr_t", "uintmax_t", + "std::uint16_t", "std::uint32_t", "std::size_t", "std::uintptr_t" + }; + static const QSet kSigned = { + "char", "signed char", "signed", "short", "int", "long", "long long", + "int8_t", "int16_t", "int32_t", "ssize_t", "ptrdiff_t", "intptr_t", + "intmax_t", "std::int8_t", "std::int16_t", "std::int32_t", + "std::ptrdiff_t", "std::intptr_t" + }; + static const QSet kFloating = { "float", "long double" }; + + if (kUnsigned.contains(n)) + return "LIDL numbers are 64-bit only. Declare it `uint64_t` (LIDL " + "`uint`). " + kWidenNote; + if (kSigned.contains(n)) + return "LIDL numbers are 64-bit only. Declare it `int64_t` (LIDL " + "`int`). " + kWidenNote; + if (kFloating.contains(n)) + return "LIDL has one floating type, `float64`. Declare it `double`."; + + if (t.startsWith("std::set<") || t.startsWith("std::unordered_set<") + || t.startsWith("std::multiset<")) + return "LIDL has no set type. Declare it `std::vector` (LIDL `[T]`); " + "uniqueness is not carried on the wire, so the module has to " + "enforce it either way."; + if (t.startsWith("std::pair<") || t.startsWith("std::tuple<")) + return "LIDL has no pair or tuple. Declare a `struct` in this header — " + "it becomes a contract `type` with named fields — or, for " + "key/value data, `std::map` (LIDL `{tstr: V}`). " + "A struct is usually the right answer: positional pairs have no " + "field names for a consumer in another language to bind to."; + if (t.startsWith("std::map<") || t.startsWith("std::unordered_map<") + || t.startsWith("std::multimap<")) + return "LIDL map keys are always `tstr`. Declare it " + "`std::map` / `std::unordered_map`, or a `[T]` of a struct carrying the key as a field."; + if (t.startsWith("std::list<") || t.startsWith("std::deque<") + || t.startsWith("std::array<") || t.startsWith("std::forward_list<")) + return "LIDL's sequence type is `[T]`, spelled `std::vector`. " + "Declare it that way."; + if (t.startsWith("Q")) + return "Qt types cannot appear in a universal impl header — the " + "module's own translation units are Qt-free, and Qt is confined " + "to the generated glue. Use the std spelling (`std::string`, " + "`std::vector`, `std::map`) or the untyped " + "`LogosMap` / `LogosList`."; + if (t.endsWith("*") || t.endsWith("&&")) + return "A pointer or rvalue reference has no wire form. Pass the value " + "(by value or `const T&`), or a `struct` declared in this " + "header."; + + return "The recognised spellings are: `bool`, `int64_t`, `uint64_t`, " + "`double`, `std::string`, `std::vector` (bytes), " + "`std::optional`, `std::vector`, `std::map`, " + "`std::unordered_map`, `LogosMap` / `LogosList` / " + "`nlohmann::json` (untyped JSON), `StdLogosResult` and `void` as " + "returns, plus any `struct` declared in this header. Rewrite the " + "declaration with one of them, or declare a struct for it."; +} + +// `context` names the declaration being typed ("method 'foo': parameter 'bar'") +// and `declared` the full spelling on it, so a nested offender reports both the +// element that has no LIDL type and the declaration that carries it. `record` is +// set only while typing a struct's fields, so a diagnostic can be withdrawn when +// the struct turns out never to reach the contract. +// +// `nameEmitted` marks the slots whose C++ spelling the generator WRITES OUT into +// code the author's own declaration has to match — a record field's codec, an +// event's generated body. In those the derived spelling is a constraint on the +// author; everywhere else the generated code only has to consume or produce a +// value, and can adapt to whatever the author declared. +static TypeExpr cppTypeToLidl(const QString& raw, const QString& context = QString(), + const QString& declared = QString(), + const QString& record = QString(), + bool nameEmitted = false) +{ + // Normalize: strip const, &, leading/trailing whitespace + QString t = normalizeCppSpelling(raw); // Primitives if (t == "bool") return { TypeExpr::Primitive, "bool", {} }; @@ -113,7 +251,7 @@ static TypeExpr cppTypeToLidl(const QString& raw) // [{tstr: int}]. Without it the element list above was exhaustive and // every other vector fell all the way through to the opaque `any`, // which then encoded a record as a LogosMap. - return { TypeExpr::Array, "", { cppTypeToLidl(inner) } }; + return { TypeExpr::Array, "", { cppTypeToLidl(inner, context, declared, record, nameEmitted) } }; } // Qt collection types — pass through directly (non-std-convertible) @@ -134,17 +272,15 @@ static TypeExpr cppTypeToLidl(const QString& raw) // The alias spelled out. LogosMap / LogosList ARE nlohmann::json, and real // modules write the underlying name — test_fullapi_cpp's `echoAny` / // `fireAnyEvent` / `anyEvent`, and both full_api interface headers, all - // declare `nlohmann::json`. It reaches `any` today ONLY through the fallback - // at the bottom of this function, so naming it here is a PREREQUISITE for + // declare `nlohmann::json`. It reached `any` ONLY through the fallback at + // the bottom of this function, so naming it here is a PREREQUISITE for // turning that fallback into an error: without this branch the whole // cross-language conformance chain stops building. // // Mapped to the bare `any` primitive rather than LogosMap's `{tstr: any}` / // LogosList's `[any]`: `nlohmann::json` is an untyped value of ANY kind, not // specifically an object or an array. That is the type the fallback already - // produces for it, so nothing about any published contract moves — this - // commit is deliberately output-neutral, and the next parser commit relies - // on that to tell a legitimate `any` from a silent admission. + // produced for it, so nothing about the published contract moves. if (t == "nlohmann::json" || t == "json") return { TypeExpr::Primitive, "any", {} }; @@ -153,12 +289,40 @@ static TypeExpr cppTypeToLidl(const QString& raw) if (t == "StdLogosResult") return { TypeExpr::Primitive, "result", {} }; - // std::map -> {tstr: T}. Absent before, so a typed map was - // unspellable header-first and fell through to `any`. - static QRegularExpression mapRe("^std::map\\s*<\\s*std::string\\s*,\\s*(.+)\\s*>$"); + // std::map / std::unordered_map -> {tstr: T}. Absent before, + // so a typed map was unspellable header-first and fell through to `any`. + // + // Both containers, because logos_codec.h specializes Codec for both and they + // are the same wire shape — a JSON object. Only the KEY is constrained: a + // non-`std::string` key falls through to the unsupported report below, since + // `{tstr: T}` is the only map LIDL has. + static QRegularExpression mapRe( + "^std::(?:unordered_)?map\\s*<\\s*std::string\\s*,\\s*(.+)\\s*>$"); QRegularExpressionMatch mm = mapRe.match(t); if (mm.hasMatch()) { - TypeExpr val = cppTypeToLidl(mm.captured(1).trimmed()); + // ...with one boundary. In a `nameEmitted` slot the generator WRITES the + // spelling out — a record field's codec says `Codec>` and + // an event's generated body repeats the parameter list the author + // declared. It has to pick one of the two container names there, and + // picking the wrong one is a compile error in code the author never + // wrote. Method parameters and returns have no such constraint: they go + // through logos::JsonArg / deduced logos::toJson, which instantiate with + // whatever the author declared. + if (t.startsWith("std::unordered_map") && nameEmitted && !context.isEmpty()) { + UnsupportedSpelling u; + u.context = context; + u.record = record; + u.declared = declared.isEmpty() ? t : declared; + u.offending = t; + u.hint = "`{tstr: T}` has two C++ spellings and this slot's spelling " + "is written into generated code your own declaration has to " + "match, so it can only be one of them: declare it " + "`std::map`. (A method parameter or return " + "may use either container — those are decoded and encoded " + "through your declared type, not a named one.)"; + g_unsupported.append(u); + } + TypeExpr val = cppTypeToLidl(mm.captured(1).trimmed(), context, declared, record, nameEmitted); return { TypeExpr::Map, "", { {TypeExpr::Primitive, "tstr", {}}, val } }; } @@ -174,7 +338,7 @@ static TypeExpr cppTypeToLidl(const QString& raw) static QRegularExpression optRe("^std::optional\\s*<\\s*(.+)\\s*>$"); QRegularExpressionMatch om = optRe.match(t); if (om.hasMatch()) { - TypeExpr inner = cppTypeToLidl(om.captured(1).trimmed()); + TypeExpr inner = cppTypeToLidl(om.captured(1).trimmed(), context, declared, record, nameEmitted); // std::optional> has NO LIDL type. // // `?T` is two-state, and optionality is idempotent under that rule — so @@ -200,7 +364,34 @@ static TypeExpr cppTypeToLidl(const QString& raw) if (g_recordNames.contains(t)) return { TypeExpr::Named, t.toStdString(), {} }; - // Fallback: treat as opaque + // NOTHING above matched: this spelling has no LIDL type. + // + // It used to return the opaque `any` right here, silently. `any` is admitted + // by every backend gate, so the declaration was accepted and the generated + // dispatch handed the raw nlohmann::json to the author's parameter with no + // `logos::fromJson<>` and no check — the one hole left open after #113-#122 + // closed it for every TYPED slot. A `std::vector` parameter + // published `[any]` and worked by accident; a + // `std::vector>` published `[any]` and + // shipped raw binary through a UTF-8 string. + // + // The return value is UNCHANGED (`any`) on purpose: mapping and diagnosis + // are separate concerns. A diagnostic that is later withdrawn — a helper + // struct that never reaches the contract, a reserved lifecycle hook — must + // leave the emitted output byte-identical to what it was. + // + // An empty spelling is not a C++ type at all, it is this line-based parser + // failing to find one (a macro, a member initialiser). Reporting "'' has no + // LIDL type" would be noise, so it keeps the old behaviour. + if (!t.isEmpty() && !context.isEmpty()) { + UnsupportedSpelling u; + u.context = context; + u.record = record; + u.declared = declared.isEmpty() ? t : declared; + u.offending = t; + u.hint = unsupportedHint(t); + g_unsupported.append(u); + } return { TypeExpr::Primitive, "any", {} }; } @@ -236,6 +427,9 @@ static std::vector scanForRecords(const QStringList& lines) TypeDecl td; td.name = om.captured(1).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; @@ -252,10 +446,15 @@ static std::vector scanForRecords(const QStringList& lines) if (!fm.hasMatch()) continue; FieldDecl fd; fd.name = fm.captured(2).toStdString(); - fd.type = cppTypeToLidl(fm.captured(1).trimmed()); + 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 (!td.fields.empty()) out.push_back(td); + else while (g_unsupported.size() > diagMark) g_unsupported.removeLast(); } return out; } @@ -313,7 +512,11 @@ static void keepOnlyReferencedRecords(ModuleDecl& module) // Parse a single method declaration line // --------------------------------------------------------------------------- -static bool parseMethodLine(const QString& line, MethodDecl& out) +// `kind` is "method" or "event" — it only labels the diagnostics an unsupported +// C++ spelling produces, so the report matches the section the declaration was +// written in rather than the function that happens to parse both. +static bool parseMethodLine(const QString& line, MethodDecl& out, + const QString& kind = "method") { // Find the parameter list: everything between the last '(' and ')' int parenOpen = -1; @@ -364,7 +567,9 @@ static bool parseMethodLine(const QString& line, MethodDecl& out) return false; out.name = methodName.toStdString(); QString retTypeStr = stripDeclarationSpecifiers(prefix.left(nameStart).trimmed()); - out.returnType = cppTypeToLidl(retTypeStr); + out.returnType = cppTypeToLidl( + retTypeStr, QString("%1 '%2': return type").arg(kind, methodName), retTypeStr, + QString(), /*nameEmitted=*/kind == "event"); // Flag methods whose impl returns LogosMap / LogosList so the generator // can emit nlohmann→Qt conversion code in the glue layer. out.jsonReturn = (retTypeStr == "LogosMap" || retTypeStr == "LogosList"); @@ -402,8 +607,13 @@ static bool parseMethodLine(const QString& line, MethodDecl& out) if (pNameStart >= pNameEnd) continue; ParamDecl pd; - pd.name = p.mid(pNameStart, pNameEnd - pNameStart).toStdString(); - pd.type = cppTypeToLidl(p.left(pNameStart)); + const QString pName = p.mid(pNameStart, pNameEnd - pNameStart); + const QString pSpelling = p.left(pNameStart).trimmed(); + pd.name = pName.toStdString(); + pd.type = cppTypeToLidl( + p.left(pNameStart), + QString("%1 '%2': parameter '%3'").arg(kind, methodName, pName), + pSpelling, QString(), /*nameEmitted=*/kind == "event"); out.params.push_back(pd); } } @@ -430,10 +640,15 @@ ImplParseResult parseImplHeader(const QString& headerPath, { ImplParseResult result; - // Both file-statics are per-parse state: one process generates for more than - // one module. Cleared here rather than next to their first use because the - // metadata's event params are typed before the header is even read. + // All three file-statics are 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_recordNames.clear(); + + QJsonArray metadataEvents; // --- Read metadata.json --- { @@ -457,24 +672,12 @@ ImplParseResult parseImplHeader(const QString& headerPath, for (const QString& depName : dependencyNames(deps)) result.module.depends.push_back(depName.toStdString()); - // Read events declared in metadata.json - QJsonArray events = obj.value("events").toArray(); - for (const QJsonValue& ev : events) { - QJsonObject evObj = ev.toObject(); - EventDecl ed; - ed.name = evObj.value("name").toString().toStdString(); - ed.description = evObj.value("description").toString().toStdString(); - QJsonArray params = evObj.value("params").toArray(); - for (const QJsonValue& pv : params) { - QJsonObject po = pv.toObject(); - ParamDecl pd; - pd.name = po.value("name").toString().toStdString(); - pd.type = cppTypeToLidl(po.value("type").toString()); - ed.params.push_back(pd); - } - if (!ed.name.empty()) - result.module.events.push_back(ed); - } + // Events declared in metadata.json. Only READ here — their parameter + // types are C++ spellings like any other, and typing them requires the + // record set, which does not exist until the header has been scanned. + // They used to be typed right here, against whatever g_recordNames the + // PREVIOUS module's parse left behind. + metadataEvents = obj.value("events").toArray(); } // --- Read and parse header --- @@ -546,6 +749,31 @@ ImplParseResult parseImplHeader(const QString& headerPath, g_recordNames.clear(); result.module.types = scanForRecords(lines); + // Now the record set exists, the metadata-declared events can be typed. They + // stay AHEAD of the header's `logos_events:` events, as they always were. + for (const QJsonValue& ev : metadataEvents) { + QJsonObject evObj = ev.toObject(); + EventDecl ed; + ed.name = evObj.value("name").toString().toStdString(); + ed.description = evObj.value("description").toString().toStdString(); + const QJsonArray params = evObj.value("params").toArray(); + for (const QJsonValue& pv : params) { + QJsonObject po = pv.toObject(); + ParamDecl pd; + const QString pName = po.value("name").toString(); + const QString pType = po.value("type").toString(); + pd.name = pName.toStdString(); + pd.type = cppTypeToLidl( + pType, + QString("event '%1': parameter '%2' (declared in metadata.json)") + .arg(evObj.value("name").toString(), pName), + pType, QString(), /*nameEmitted=*/true); + ed.params.push_back(pd); + } + if (!ed.name.empty()) + result.module.events.push_back(ed); + } + // State machine: find "class ", then collect declarations. // `InLogosEvents` is entered by the literal `logos_events:` token // (mirrors Qt's `signals:`) — methods declared there are parsed as @@ -705,7 +933,7 @@ ImplParseResult parseImplHeader(const QString& headerPath, if (line.endsWith(';')) { QString decl = line.left(line.size() - 1).trimmed(); MethodDecl md; - if (parseMethodLine(decl, md)) { + if (parseMethodLine(decl, md, "event")) { EventDecl ed; ed.name = md.name; ed.params = md.params; @@ -732,6 +960,10 @@ ImplParseResult parseImplHeader(const QString& headerPath, if (line.endsWith(';')) { QString decl = line.left(line.size() - 1).trimmed(); MethodDecl md; + // Withdraw the declaration's diagnostics if it turns out to be a + // reserved lifecycle hook: it is not part of the contract, so an + // unsupported spelling in it is not a contract defect. + const int diagMark = g_unsupported.size(); if (parseMethodLine(decl, md)) { // LogosModuleContext lifecycle hooks / context accessors are // framework plumbing, not part of the module's API contract. @@ -748,6 +980,9 @@ ImplParseResult parseImplHeader(const QString& headerPath, if (!reserved.contains(qs(md.name))) { md.description = joinDocLines(pendingDoc).toStdString(); result.module.methods.push_back(md); + } else { + while (g_unsupported.size() > diagMark) + g_unsupported.removeLast(); } } } @@ -762,6 +997,49 @@ done: // contract types. keepOnlyReferencedRecords(result.module); + // A C++ spelling with no LIDL type is a BUILD ERROR, not a silent `any`. + // + // Reported after keepOnlyReferencedRecords so a helper struct that never + // reaches the contract cannot fail the build: publishing is what makes a + // declaration's type a promise, and an internal struct promises nothing. + { + std::set published; + for (const TypeDecl& td : result.module.types) published.insert(td.name); + + QStringList reports; + QSet seen; + for (const UnsupportedSpelling& u : g_unsupported) { + if (!u.record.isEmpty() && !published.count(u.record.toStdString())) + continue; // struct dropped: not part of the contract + QString line = " " + u.context; + // "declared X, whose element Y" only when Y really is nested inside + // X — not when the two differ by a `const` and an `&`. + if (normalizeCppSpelling(u.declared) != u.offending) + line += QString(" is declared `%1`, whose element `%2` has no " + "LIDL type.\n ").arg(u.declared, u.offending); + else + line += QString(" is `%1`, which has no LIDL type.\n ") + .arg(u.offending); + line += u.hint; + if (seen.contains(line)) continue; + seen.insert(line); + reports << line; + } + if (!reports.isEmpty()) { + result.error = + headerPath + ": " + QString::number(reports.size()) + + (reports.size() == 1 ? " declaration uses" : " declarations use") + + " a C++ type that has no LIDL type.\n\n" + + reports.join("\n\n") + + "\n\nEach of these used to be published as the opaque `any`, with no " + "diagnostic. `any` is admitted by every backend gate, so the generated " + "dispatch handed the raw JSON straight to the parameter with no decode " + "and no check — the value either converted by luck, threw at call time, " + "or went onto the wire in a form no other language decodes.\n"; + return result; + } + } + if (!g_unmappableSpellings.isEmpty()) { g_unmappableSpellings.removeDuplicates(); err << "Warning: " << headerPath << ": " << g_unmappableSpellings.join(", ") diff --git a/tests/experimental/test_impl_header_parser.cpp b/tests/experimental/test_impl_header_parser.cpp index 1fc3bb2..63857f2 100644 --- a/tests/experimental/test_impl_header_parser.cpp +++ b/tests/experimental/test_impl_header_parser.cpp @@ -743,3 +743,190 @@ TEST_F(ImplHeaderParserTest, NlohmannJsonIsAnyByName) ASSERT_EQ(r.module.events.size(), 1u); EXPECT_EQ(r.module.events[0].params[0].type.name, "any"); } + +// --------------------------------------------------------------------------- +// A C++ spelling with no LIDL type is a BUILD ERROR, not a silent `any`. +// +// cppTypeToLidl used to end with `// Fallback: treat as opaque` -> `any`, and +// `any` is admitted by every backend gate. So an unrecognised spelling was +// accepted in silence, published as `any`, and dispatched as a raw +// `lidlImpl().f(args.at(0))` with no decode and no check. +// --------------------------------------------------------------------------- + +TEST_F(ImplHeaderParserTest, NarrowNumericIsRejectedWithTheWidening) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "class ProbeImpl {\n" + "public:\n" + " int64_t f(uint32_t depth);\n" + "};\n"); + ASSERT_FALSE(hp.isEmpty()); + + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_TRUE(r.hasError()) << "uint32_t was admitted as `any`"; + // Names the declaration, the offending type, and what to write instead. + EXPECT_TRUE(r.error.contains("method 'f': parameter 'depth'")) << r.error.toStdString(); + EXPECT_TRUE(r.error.contains("`uint32_t`")) << r.error.toStdString(); + EXPECT_TRUE(r.error.contains("`uint64_t`")) << r.error.toStdString(); +} + +// The offender may be nested. Report BOTH: the element with no LIDL type, and +// the declaration that carries it. +TEST_F(ImplHeaderParserTest, NestedOffenderNamesTheDeclarationToo) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "class ProbeImpl {\n" + "public:\n" + " int64_t f(const std::vector& instruction);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_TRUE(r.hasError()); + EXPECT_TRUE(r.error.contains("const std::vector&")) << r.error.toStdString(); + EXPECT_TRUE(r.error.contains("`uint32_t`")) << r.error.toStdString(); +} + +// Each family gets a hint that names a replacement. A diagnostic without one +// just moves the guesswork. +TEST_F(ImplHeaderParserTest, EachUnsupportedFamilyNamesItsReplacement) +{ + struct Case { const char* decl; const char* mentions; }; + const Case cases[] = { + {"int64_t f(float v);", "`double`"}, + {"int64_t f(uint8_t v);", "std::vector"}, + {"int64_t f(size_t v);", "`uint64_t`"}, + {"int64_t f(const std::set& v);", "std::vector"}, + {"int64_t f(const std::pair& v);", "struct"}, + {"int64_t f(const std::map& v);", "`tstr`"}, + }; + for (const Case& c : cases) { + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + QString("class ProbeImpl {\npublic:\n %1\n};\n").arg(c.decl)); + QString e; + QTextStream es(&e); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", es); + ASSERT_TRUE(r.hasError()) << c.decl; + EXPECT_TRUE(r.error.contains(c.mentions)) + << c.decl << "\n" << r.error.toStdString(); + } +} + +// std::unordered_map is the second C++ spelling of `{tstr: T}`. +// logos_codec.h has always specialized Codec for it; the parser had not, so it +// published `any`. +TEST_F(ImplHeaderParserTest, UnorderedMapIsAStringKeyedMap) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "class ProbeImpl {\n" + "public:\n" + " int64_t f(const std::unordered_map& m);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + ASSERT_EQ(r.module.methods.size(), 1u); + const TypeExpr& t = r.module.methods[0].params[0].type; + EXPECT_EQ(t.kind, TypeExpr::Map); + ASSERT_EQ(t.elements.size(), 2u); + EXPECT_EQ(t.elements[0].name, "tstr"); + EXPECT_EQ(t.elements[1].name, "tstr"); +} + +// ...except in the two slots whose C++ spelling the generator WRITES OUT — a +// record field's codec and an event's generated body. There it has to pick one +// container name, and picking the wrong one is a compile error in code the +// author never wrote. Say so at the declaration instead. +TEST_F(ImplHeaderParserTest, UnorderedMapIsRejectedWhereTheSpellingIsEmitted) +{ + for (const char* body : { + "struct Rec {\n" + " std::unordered_map m;\n" + "};\n" + "class ProbeImpl {\npublic:\n Rec echo(const Rec& v);\n};\n", + + "class ProbeImpl {\n" + "public:\n" + " bool fire();\n" + "logos_events:\n" + " void changed(const std::unordered_map& m);\n" + "};\n"}) { + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, body); + QString e; + QTextStream es(&e); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", es); + ASSERT_TRUE(r.hasError()) << body; + EXPECT_TRUE(r.error.contains("std::map")) << r.error.toStdString(); + } +} + +// A helper struct the API never mentions is dropped from the contract, so an +// unsupported spelling INSIDE it promises nothing and must not fail the build. +// Publishing is what makes a declaration's type a promise. +TEST_F(ImplHeaderParserTest, UnreferencedHelperStructDoesNotFailTheBuild) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "struct PendingAction {\n" + " uint32_t attempts;\n" + "};\n" + "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(); + EXPECT_TRUE(r.module.types.empty()); + + // ...and the same struct DOES fail once a method publishes it. + QTemporaryDir dir2; + ASSERT_TRUE(dir2.isValid()); + const QString hp2 = probeHeader(dir2, + "struct PendingAction {\n" + " uint32_t attempts;\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " PendingAction f(int64_t n);\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 'PendingAction': field 'attempts'")) + << r2.error.toStdString(); +} + +// The reserved LogosModuleContext hooks are framework plumbing, not contract. +// They are dropped after parsing, so their spellings are not a contract defect. +TEST_F(ImplHeaderParserTest, ReservedHookSpellingsAreNotContractDefects) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "class ProbeImpl {\n" + "public:\n" + " void onContextReady(uint32_t generation);\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(); + ASSERT_EQ(r.module.methods.size(), 1u); + EXPECT_EQ(r.module.methods[0].name, "f"); +}