diff --git a/CMakeLists.txt b/CMakeLists.txt index e6e37ef..d48ea8a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -32,6 +32,7 @@ add_subdirectory(frontends/xml) add_subdirectory(frontends/dsl) add_subdirectory(roads/opendrive) add_subdirectory(tools/scena-run) +add_subdirectory(tools/scena-check) if(SCN_BUILD_CAPI) add_subdirectory(capi) diff --git a/docs/user-guide/index.md b/docs/user-guide/index.md index fb9318e..4d4a7c8 100644 --- a/docs/user-guide/index.md +++ b/docs/user-guide/index.md @@ -58,6 +58,9 @@ The user guide grows sprint by sprint along the - [`scena-run`](scena-run.md) — the headless CLI: options and exit codes, the round-trip-exact trace format, replaying a host-controlled entity, and the golden suite harness. +- [`scena-check`](scena-check.md) — the OpenSCENARIO DSL checker: options and + exit codes, the diagnostic format, resolving imports and search paths, and + what "checked clean" does and does not cover. - [Python quickstart](python.md) — installing, loading and running a scenario, building one in memory, observing a run, driving entities from the host with a `SimulatorGateway` subclass, the GIL and reentrancy policy, and the parity diff --git a/docs/user-guide/scena-check.md b/docs/user-guide/scena-check.md new file mode 100644 index 0000000..582bcb3 --- /dev/null +++ b/docs/user-guide/scena-check.md @@ -0,0 +1,122 @@ +# `scena-check` — checking OpenSCENARIO DSL + +`scena-check` loads an OpenSCENARIO DSL file, follows its imports, and reports +what the checker finds. It does not execute anything: DSL execution is a later +milestone, and a file that checks clean here is one the frontend understood, not +one that will necessarily run. + +``` +scena-check my_scenario.osc +``` + +``` +my_scenario.osc: ok, 412 types across 3 files +``` + +## Options + +| Option | Meaning | +| --- | --- | +| `-I`, `--search-path ` | A directory to resolve module imports against. Repeatable; searched in the order given. | +| `--no-standard-library` | Do not load the bundled `osc.standard` library. | +| `--strict` | Exit non-zero when there are warnings, not only errors. | +| `--quiet` | Do not print diagnostics. | +| `-h`, `--help` | Usage text. | + +## Exit codes + +| Code | Meaning | +| --- | --- | +| `0` | The file checked. There may still have been warnings — see `--strict`. | +| `2` | The command line was wrong. | +| `3` | The source did not check: it has errors, an import did not resolve, or `--strict` was given and there were warnings. | +| `4` | The input could not be read. | + +`3` and `4` are deliberately different. Scena's status model separates a defect +in the content from the host handing the library something it cannot use, and +the exit codes follow that line: a file full of type errors is your scenario's +problem, an unreadable path is your invocation's. A build script can act on the +difference. + +## Diagnostics + +One diagnostic per line, in the same shape [`scena-run`](scena-run.md) prints, so +a script that parses one tool's output can parse the other's: + +``` +error: my_scenario.osc:14:9: unknown type 'vehicel' (§7.7.4.2) +warning: my_scenario.osc:31:5: this constraint needs a solver; v0.0.1 resolves fixed values only (§7.3.11, ADR-0004) +``` + +The file is always named, including for a diagnostic that came from an imported +file rather than from the one you passed — a program spans every file its root +imported, so a line number on its own would not locate anything. + +Every diagnostic cites the section of the standard it comes from. Unlike the XML +frontend's, a DSL diagnostic carries no bracketed rule identifier: the DSL +standard defines no `asam.net:` rule ids, so the section reference in the message +is the citation. + +Error recovery is contractual. A malformed declaration does not cost its +siblings, so one run reports as much as it can rather than stopping at the first +problem. + +## Imports + +Both of the reference forms in the standard work. + +A **file reference** is resolved relative to the file that wrote it: + +``` +import "shared/common.osc" +``` + +A **module reference** maps `a.b.c` to `a/b/c.osc` and is looked for under each +`--search-path`, in the order you gave them: + +``` +import shared.common +``` + +``` +scena-check main.osc -I ./lib -I ./vendor/lib +``` + +A file referenced twice is imported once, keyed on its canonical path. A diamond +therefore declares its shared types once, and an import cycle terminates instead +of erroring. + +References beginning with `osc` are reserved by the standard and never reach the +search path; an unknown one is reported rather than looked up as a module of +yours. + +## The standard library + +The `osc.standard` library is bundled — it is not files on disk, so there is +nothing to install or point at. Its physical types are available without an +import, because they are what gives a literal like `30kph` a type at all; the +domain model needs an explicit import: + +``` +import osc.standard.all +namespace demo use std, stdtypes + +scenario overtake: + ego: vehicle + target: lane + keep(target.lane_type == lane_type!driving) + keep(target.width == 3.5m) +``` + +`--no-standard-library` turns even the implicit part off, which is useful when +checking a file that is meant to stand alone. + +## What "checked clean" covers + +The checker resolves names, types every expression, and validates constraints +and coverage declarations against the standard's rules. Constraints it cannot +resolve to fixed values are reported as warnings rather than errors — solving +them needs a constraint solver, which is out of scope for v0.0.1 — so a clean +exit means "well-formed and understood", not "solved". + +Run with `--strict` in CI if you want those warnings to fail the build. diff --git a/frontends/dsl/include/scena/dsl/expression.h b/frontends/dsl/include/scena/dsl/expression.h index d3980e0..271202c 100644 --- a/frontends/dsl/include/scena/dsl/expression.h +++ b/frontends/dsl/include/scena/dsl/expression.h @@ -35,6 +35,10 @@ struct ExpressionContext { /// The namespace the expression was written in, for resolving type names in /// `is()`, `as()` and enum references. std::string name_space; + /// The source file the expression was written in. A `Program` spans every + /// file its root imported, so a line number alone does not locate a + /// diagnostic — `scena-check` needs the file to point at. + std::string file; /// The namespaces the enclosing `namespace ... use` clause makes visible. std::vector uses; /// Names bound by the construct the expression sits in rather than by the diff --git a/frontends/dsl/src/expression.cpp b/frontends/dsl/src/expression.cpp index 03c2323..dddd2c9 100644 --- a/frontends/dsl/src/expression.cpp +++ b/frontends/dsl/src/expression.cpp @@ -37,11 +37,17 @@ namespace { } /// Reports and returns kInvalidType, so a caller can `return fail(...)`. -TypeId fail(DiagnosticSink& sink, const SourceRange& at, std::string message) { +/// +/// `file` is threaded through rather than left to the caller because a +/// `Program` spans every file its root imported: a line number on its own does +/// not say which file it is a line of. +TypeId fail(DiagnosticSink& sink, std::string_view file, const SourceRange& at, + std::string message) { Diagnostic diagnostic; diagnostic.severity = Severity::Error; diagnostic.code = Status::ValidationError; diagnostic.message = std::move(message); + diagnostic.location.file = file; diagnostic.location.line = at.line; diagnostic.location.column = at.column; sink.report(std::move(diagnostic)); @@ -175,7 +181,8 @@ TypeId Typer::type_name(const Expr& expression, TypeId it_type) { if (context_.self != kInvalidType) { return context_.self; } - return fail(sink_, expression.range, "'it' has no instance to refer to here (§7.4.1.3)"); + return fail(sink_, context_.file, expression.range, + "'it' has no instance to refer to here (§7.4.1.3)"); } // A parameter of the method or event the expression belongs to, which the // enclosing type's fields do not shadow. @@ -216,27 +223,28 @@ TypeId Typer::type_name(const Expr& expression, TypeId it_type) { for (const TypeId id : enums) { names += (names.empty() ? "" : ", ") + program_.types[id].name; } - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "'" + expression.text + "' is a member of more than one enum (" + names + "); write the enum name (§7.3.3)"); } // A type name, which is legal as the operand of nothing in an expression, // but naming it precisely beats "unknown name". if (lookup_type(expression.text) != kInvalidType) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "'" + expression.text + "' is a type, not a value (§7.4.1.1)"); } - return fail(sink_, expression.range, "unknown name '" + expression.text + "' (§7.4.1.1)"); + return fail(sink_, context_.file, expression.range, + "unknown name '" + expression.text + "' (§7.4.1.1)"); } TypeId Typer::type_enum_value(const Expr& expression) { const TypeId id = lookup_type(expression.type_name); if (id == kInvalidType) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "unknown enum '" + expression.type_name + "' (§7.3.3)"); } if (program_.types[id].kind != TypeKind::Enum) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "'" + expression.type_name + "' is not an enum (§7.3.3)"); } const TypeInfo& enumeration = program_.types[id]; @@ -244,7 +252,7 @@ TypeId Typer::type_enum_value(const Expr& expression) { std::any_of(enumeration.enum_members.begin(), enumeration.enum_members.end(), [&](const EnumMemberInfo& member) { return member.name == expression.text; }); if (!known) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "'" + expression.type_name + "' has no member '" + expression.text + "' (§7.3.3)"); } @@ -258,7 +266,7 @@ TypeId Typer::type_unary(const Expr& expression, TypeId it_type) { } if (expression.text == "not") { if (kind_of(operand) != TypeKind::Bool) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "'not' takes a bool, not " + spelling(operand) + " (§7.4.2.2)"); } return operand; @@ -266,7 +274,7 @@ TypeId Typer::type_unary(const Expr& expression, TypeId it_type) { // Unary minus: "the result type is the type of its operand, or int if the // operand is of type uint" (§7.4.2.3.1). if (!is_numeric(kind_of(operand))) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "unary '-' takes a numeric type, not " + spelling(operand) + " (§7.4.2.3)"); } return kind_of(operand) == TypeKind::Uint ? kind_id(TypeKind::Int) : operand; @@ -282,7 +290,8 @@ TypeId Typer::type_binary(const Expr& expression, TypeId it_type) { if (op == "and" || op == "or" || op == "=>") { if (kind_of(left) != TypeKind::Bool || kind_of(right) != TypeKind::Bool) { - return fail(sink_, expression.range, "'" + op + "' takes bool operands (§7.4.2.2)"); + return fail(sink_, context_.file, expression.range, + "'" + op + "' takes bool operands (§7.4.2.2)"); } return left; } @@ -291,14 +300,14 @@ TypeId Typer::type_binary(const Expr& expression, TypeId it_type) { // §7.4.2.7.1 / §7.4.2.8.3: the right operand is a list or a range. const TypeKind container = kind_of(right); if (container != TypeKind::List && container != TypeKind::Range) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "'in' takes a list or a range on the right (§7.4.2.4.1)"); } const TypeId element = program_.types[right].element; if (element != kInvalidType && left != element && common_numeric_type(program_, left, element) == kInvalidType && !program_.is_derived_from(left, element)) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "'in' compares " + spelling(left) + " against " + spelling(right) + " (§7.4.2.4.1)"); } @@ -312,14 +321,14 @@ TypeId Typer::type_binary(const Expr& expression, TypeId it_type) { // §7.4.2.4.1: after conversion the two types must be identical, and // physical types never convert. if (common_numeric_type(program_, left, right) == kInvalidType) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "cannot compare " + spelling(left) + " with " + spelling(right) + " (§7.4.2.4.1)"); } return kind_id(TypeKind::Bool); } if (op != "==" && op != "!=") { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "'" + op + "' applies to numeric expressions; " + spelling(left) + " is not one (§7.4.2.4.2)"); } @@ -327,7 +336,7 @@ TypeId Typer::type_binary(const Expr& expression, TypeId it_type) { // one must inherit from the other". if (left != right && !program_.is_derived_from(left, right) && !program_.is_derived_from(right, left)) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "cannot compare " + spelling(left) + " with " + spelling(right) + " (§7.4.2.4.2)"); } @@ -336,7 +345,8 @@ TypeId Typer::type_binary(const Expr& expression, TypeId it_type) { // Arithmetic (§7.4.2.3). if (!is_numeric(kind_of(left)) || !is_numeric(kind_of(right))) { - return fail(sink_, expression.range, "'" + op + "' takes numeric operands (§7.4.2.3)"); + return fail(sink_, context_.file, expression.range, + "'" + op + "' takes numeric operands (§7.4.2.3)"); } if (kind_of(left) == TypeKind::Physical || kind_of(right) == TypeKind::Physical) { // §7.4.2.3.1: "Physical types are not converted." Addition and @@ -347,7 +357,7 @@ TypeId Typer::type_binary(const Expr& expression, TypeId it_type) { // was never declared. if (op == "+" || op == "-" || op == "%") { if (left != right) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "'" + op + "' needs the same physical type " "on both sides; " + @@ -357,7 +367,7 @@ TypeId Typer::type_binary(const Expr& expression, TypeId it_type) { return left; } if (kind_of(left) == TypeKind::Physical && kind_of(right) == TypeKind::Physical) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "'" + op + "' over two physical types would need a type that is not " "declared; write the result's type (§7.4.2.3.1)"); @@ -366,7 +376,7 @@ TypeId Typer::type_binary(const Expr& expression, TypeId it_type) { } const TypeId common = common_numeric_type(program_, left, right); if (common == kInvalidType) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "cannot apply '" + op + "' to " + spelling(left) + " and " + spelling(right) + " (§7.4.2.3.1)"); } @@ -381,7 +391,7 @@ TypeId Typer::type_postfix(const Expr& expression, TypeId it_type) { } const TypeId target = lookup_type(expression.type_name); if (target == kInvalidType) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "unknown type '" + expression.type_name + "' (§7.4.2.5)"); } if (expression.kind == ExprKind::TypeTest) { @@ -394,7 +404,7 @@ TypeId Typer::type_postfix(const Expr& expression, TypeId it_type) { kind_of(operand) == TypeKind::Enum || kind_of(target) == TypeKind::Enum; if (!related) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "cannot convert " + spelling(operand) + " to " + spelling(target) + " (§7.4.2.6)"); } @@ -408,12 +418,12 @@ TypeId Typer::type_postfix(const Expr& expression, TypeId it_type) { return kInvalidType; } if (kind_of(container) != TypeKind::List) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "indexing applies to a list, not " + spelling(container) + " (§7.4.2.7.2)"); } const TypeKind index_kind = kind_of(index); if (index_kind != TypeKind::Int && index_kind != TypeKind::Uint) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "a list index is an integer expression (§7.4.2.7.2)"); } return program_.types[container].element; @@ -429,17 +439,18 @@ TypeId Typer::type_postfix(const Expr& expression, TypeId it_type) { return field->type; } if (program_.find_method(owner, expression.text) != nullptr) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "'" + expression.text + "' is a method; call it (§7.4.2.1)"); } - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, spelling(owner) + " has no field '" + expression.text + "' (§7.4.1.1)"); } TypeId Typer::type_call(const Expr& expression, TypeId it_type) { const Expr& callee = *expression.operands[0]; if (callee.kind != ExprKind::FieldAccess && callee.kind != ExprKind::Name) { - return fail(sink_, expression.range, "this expression cannot be called (§7.4.2.1)"); + return fail(sink_, context_.file, expression.range, + "this expression cannot be called (§7.4.2.1)"); } // Built-in list and range operators are methods on the aggregate, not on a @@ -458,7 +469,7 @@ TypeId Typer::type_call(const Expr& expression, TypeId it_type) { } if (is_member_evaluation(name)) { if (expression.arguments.size() != 1) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "'" + name + "' takes one expression (§7.4.2.7.3)"); } // `it` is the current member inside the argument (§7.4.2.7.3). @@ -470,7 +481,7 @@ TypeId Typer::type_call(const Expr& expression, TypeId it_type) { return aggregate(TypeKind::List, argument); } if (kind_of(argument) != TypeKind::Bool) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "'" + name + "' takes a bool expression (§7.4.2.7.3)"); } if (name == "filter") { @@ -486,7 +497,7 @@ TypeId Typer::type_call(const Expr& expression, TypeId it_type) { } const MethodInfo* method = program_.find_method(owner, name); if (method == nullptr) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, spelling(owner) + " has no method '" + name + "' (§7.4.2.1)"); } // Copied out before typing the arguments: that can intern an aggregate @@ -494,7 +505,7 @@ TypeId Typer::type_call(const Expr& expression, TypeId it_type) { const TypeId returns = method->return_type; const std::size_t arity = method->parameters.size(); if (expression.arguments.size() > arity) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "'" + name + "' takes " + std::to_string(arity) + " arguments (§7.4.2.1)"); } for (const Argument& argument : expression.arguments) { @@ -510,11 +521,12 @@ TypeId Typer::type_call(const Expr& expression, TypeId it_type) { return type_range(expression, it_type); } if (context_.self == kInvalidType) { - return fail(sink_, expression.range, "unknown method '" + callee.text + "' (§7.4.2.1)"); + return fail(sink_, context_.file, expression.range, + "unknown method '" + callee.text + "' (§7.4.2.1)"); } const MethodInfo* method = program_.find_method(context_.self, callee.text); if (method == nullptr) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, spelling(context_.self) + " has no method '" + callee.text + "' (§7.4.2.1)"); } const TypeId returns = method->return_type; @@ -557,14 +569,15 @@ TypeId Typer::type_list(const Expr& expression, TypeId it_type) { candidate = program_.types[candidate].base; } if (candidate == kInvalidType) { - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "a list needs a common element type; " + spelling(common) + " and " + spelling(member_type) + " have none (§7.4.2.7.4)"); } common = candidate; } if (common == kInvalidType) { - return fail(sink_, expression.range, "an empty list has no element type (§7.4.2.7.4)"); + return fail(sink_, context_.file, expression.range, + "an empty list has no element type (§7.4.2.7.4)"); } return aggregate(TypeKind::List, common); } @@ -580,7 +593,8 @@ TypeId Typer::type_range(const Expr& expression, TypeId it_type) { low = arguments[0].value.get(); high = arguments[1].value.get(); } else { - return fail(sink_, expression.range, "a range takes two bounds (§7.4.2.8.1)"); + return fail(sink_, context_.file, expression.range, + "a range takes two bounds (§7.4.2.8.1)"); } const TypeId left = type_of(*low, it_type); const TypeId right = type_of(*high, it_type); @@ -588,14 +602,15 @@ TypeId Typer::type_range(const Expr& expression, TypeId it_type) { return kInvalidType; } if (!is_numeric(kind_of(left)) || !is_numeric(kind_of(right))) { - return fail(sink_, expression.range, "a range is built over a numeric type (§7.4.2.8.1)"); + return fail(sink_, context_.file, expression.range, + "a range is built over a numeric type (§7.4.2.8.1)"); } const TypeId common = common_numeric_type(program_, left, right); if (common == kInvalidType) { // §7.4.2.8.1: "When constructing a range of a physical type, both // expressions must be of that physical type, with possibly different // units." - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "a range needs one type for both bounds; " + spelling(left) + " and " + spelling(right) + " differ (§7.4.2.8.1)"); } @@ -621,7 +636,8 @@ TypeId Typer::type_of(const Expr& expression, TypeId it_type) { case ExprKind::PhysicalLiteral: { const auto unit = program_.units.find(expression.text); if (unit == program_.units.end()) { - return fail(sink_, expression.range, "unknown unit '" + expression.text + "' (§7.3.4)"); + return fail(sink_, context_.file, expression.range, + "unknown unit '" + expression.text + "' (§7.3.4)"); } return unit->second.physical_type; } @@ -642,7 +658,8 @@ TypeId Typer::type_of(const Expr& expression, TypeId it_type) { return kInvalidType; } if (kind_of(condition) != TypeKind::Bool) { - return fail(sink_, expression.range, "'?' takes a bool condition (§7.4.2.9)"); + return fail(sink_, context_.file, expression.range, + "'?' takes a bool condition (§7.4.2.9)"); } if (when_true == when_false) { return when_true; @@ -657,7 +674,7 @@ TypeId Typer::type_of(const Expr& expression, TypeId it_type) { if (program_.is_derived_from(when_false, when_true)) { return when_true; } - return fail(sink_, expression.range, + return fail(sink_, context_.file, expression.range, "the arms of '?' have no common type: " + spelling(when_true) + " and " + spelling(when_false) + " (§7.4.2.9)"); } diff --git a/frontends/dsl/src/resolve.cpp b/frontends/dsl/src/resolve.cpp index 89fc3b0..3ba6ca2 100644 --- a/frontends/dsl/src/resolve.cpp +++ b/frontends/dsl/src/resolve.cpp @@ -147,6 +147,9 @@ constexpr std::array kPrimitives = { struct Scope { std::string name_space; std::vector uses; + /// The file the declaration was written in. A `Program` spans every file + /// the root imported, so a line number alone does not locate anything. + std::string file; }; struct PendingUnit { @@ -236,6 +239,9 @@ class Resolver { void reject_it(const ExprPtr& expression, std::string_view where); const std::vector& files_; + /// The file whose declarations the current pass is walking, so a + /// diagnostic can name it. Set from the pending record's scope. + std::string current_file_; Program& out_; DiagnosticSink& sink_; bool failed_ = false; @@ -256,6 +262,7 @@ void Resolver::error(const SourceRange& at, std::string message) { diagnostic.severity = Severity::Error; diagnostic.code = Status::ValidationError; diagnostic.message = std::move(message); + diagnostic.location.file = current_file_; diagnostic.location.line = at.line; diagnostic.location.column = at.column; sink_.report(std::move(diagnostic)); @@ -267,6 +274,7 @@ void Resolver::warn(const SourceRange& at, std::string message) { diagnostic.severity = Severity::Warning; diagnostic.code = Status::Ok; diagnostic.message = std::move(message); + diagnostic.location.file = current_file_; diagnostic.location.line = at.line; diagnostic.location.column = at.column; sink_.report(std::move(diagnostic)); @@ -404,6 +412,8 @@ void Resolver::declare() { // the list, exactly as §7.7.4 says: from there the ordinary rules hold. Scope scope; scope.uses.emplace_back(kStandardTypesNamespace); + scope.file = file->path; + current_file_ = file->path; for (const Declaration& declaration : file->declarations) { switch (declaration.kind) { case Declaration::Kind::Import: @@ -414,6 +424,7 @@ void Resolver::declare() { case Declaration::Kind::Namespace: { scope.name_space = declaration.name_space.name; scope.uses = declaration.name_space.uses; + scope.file = file->path; if (!is_null_namespace(scope.name_space)) { if (!file->is_standard_library && scope.name_space.rfind("std", 0) == 0) { warn(declaration.range, @@ -796,12 +807,14 @@ void Resolver::check_inheritance_cycles() { void Resolver::link() { for (const PendingUnit& unit : units_) { + current_file_ = unit.scope.file; link_unit(unit); } for (const auto& [id, declarations] : enum_declarations_) { link_enum_values(out_.types[id], declarations); } for (PendingStructured& pending : structured_) { + current_file_ = pending.scope.file; link_structured(pending); } check_inheritance_cycles(); @@ -1151,6 +1164,7 @@ void Resolver::add_members(const PendingStructured& pending) { void Resolver::check_members() { for (const PendingStructured& pending : structured_) { + current_file_ = pending.scope.file; add_members(pending); } @@ -1297,6 +1311,7 @@ void Resolver::note_unsupported(const SourceRange& at, std::string message) { diagnostic.severity = Severity::Warning; diagnostic.code = Status::UnsupportedFeature; diagnostic.message = std::move(message); + diagnostic.location.file = current_file_; diagnostic.location.line = at.line; diagnostic.location.column = at.column; sink_.report(std::move(diagnostic)); @@ -1475,10 +1490,12 @@ void Resolver::check_coverage(const CoverageDecl& coverage, const ExpressionCont void Resolver::check_expressions() { for (const PendingStructured& pending : structured_) { + current_file_ = pending.scope.file; ExpressionContext context; context.self = pending.id; context.name_space = pending.scope.name_space; context.uses = pending.scope.uses; + context.file = pending.scope.file; for (const Member& member : pending.decl->members) { switch (member.kind) { case Member::Kind::Field: { @@ -1584,8 +1601,10 @@ void Resolver::check_expressions() { field.default_value->kind == ExprKind::PhysicalLiteral) { continue; } + current_file_ = pending.scope.file; ExpressionContext context; context.name_space = pending.scope.name_space; + context.file = pending.scope.file; context.uses = pending.scope.uses; const std::string qualified = qualify(pending.scope.name_space, field.names.front()); const auto global = out_.globals.find(qualified); diff --git a/frontends/dsl/tests/dsl_import_test.cpp b/frontends/dsl/tests/dsl_import_test.cpp index 8472502..dcab4b6 100644 --- a/frontends/dsl/tests/dsl_import_test.cpp +++ b/frontends/dsl/tests/dsl_import_test.cpp @@ -397,4 +397,35 @@ TEST(DslImportTest, LoadingIsDeterministic) { EXPECT_EQ(first, second); } +TEST(DslImportTest, ADiagnosticNamesTheFileItCameFrom) { + // A Program spans every file its root imported, so a line number on its own + // does not locate anything. Both the resolver's diagnostics and the ones + // expression typing reports must carry the file they came from — without it + // `scena-check` can print a line number but not say which file it is a line + // of (p7-s5, #43). + const Tree tree; + tree.write("helper.osc", "struct helper:\n" + " v: no_such_type\n" + "export *\n"); + const auto root = tree.write("main.osc", "import \"helper.osc\"\n" + "struct probe:\n" + " w: another_missing_type\n" + " keep(it.w == unknown_name)\n"); + DiagnosticSink sink; + LoadResult loaded; + Program program; + EXPECT_EQ(scena::dsl::check_file(root, LoadOptions{}, loaded, program, sink), + Status::ValidationError); + bool named_helper = false; + bool named_root = false; + for (const scena::Diagnostic& diagnostic : sink.diagnostics()) { + EXPECT_FALSE(diagnostic.location.file.empty()) << diagnostic.message; + named_helper = + named_helper || diagnostic.location.file.find("helper.osc") != std::string::npos; + named_root = named_root || diagnostic.location.file.find("main.osc") != std::string::npos; + } + EXPECT_TRUE(named_helper) << "the imported file's error must name the imported file"; + EXPECT_TRUE(named_root) << "the root's error must name the root"; +} + } // namespace diff --git a/tools/scena-check/CMakeLists.txt b/tools/scena-check/CMakeLists.txt new file mode 100644 index 0000000..75e9c5e --- /dev/null +++ b/tools/scena-check/CMakeLists.txt @@ -0,0 +1,19 @@ +# SPDX-FileCopyrightText: 2026 Robomous +# SPDX-License-Identifier: Apache-2.0 +# scena-check: the OpenSCENARIO DSL checking CLI. +# +# A thin consumer of the public DSL loader — check_file and the diagnostic +# sink — with no language semantics of its own. It links the DSL frontend only: +# checking a .osc file needs no engine, no road backend and no XML frontend. + +add_executable(scena-check src/main.cpp) +target_link_libraries(scena-check PRIVATE scena::core scena::frontend-dsl) +target_compile_features(scena-check PRIVATE cxx_std_20) +set_target_properties(scena-check PROPERTIES CXX_EXTENSIONS OFF) +scn_set_warnings(scena-check) +scn_set_fp_strictness(scena-check) +scn_enable_sanitizers(scena-check) + +if(SCN_BUILD_TESTS) + add_subdirectory(tests) +endif() diff --git a/tools/scena-check/src/main.cpp b/tools/scena-check/src/main.cpp new file mode 100644 index 0000000..14baab2 --- /dev/null +++ b/tools/scena-check/src/main.cpp @@ -0,0 +1,215 @@ +/* + * Copyright 2026 Robomous + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// +// scena-check — load an OpenSCENARIO DSL file, resolve its imports and report +// what the checker finds. A thin consumer of `scena::dsl::check_file`, with no +// language semantics of its own (p7-s5, #43). +// +// It does not execute anything: DSL execution is P8. A file that checks clean +// here is one the frontend understood, not one that will necessarily run. +// + +#include +#include +#include +#include +#include +#include +#include + +#include "scena/diagnostic.h" +#include "scena/dsl/load.h" +#include "scena/dsl/types.h" +#include "scena/status.h" + +namespace { + +/// Exit codes. Distinct per failure so a script can branch on them, and split +/// along the same line the Status model draws: a content defect in the input is +/// not the same thing as the host handing us something unusable. +enum ExitCode : int { + kOk = 0, ///< the file checked clean + kUsage = 2, ///< bad command line + kCheckFailed = 3, ///< the DSL source has errors (or warnings under --strict) + kInputFailed = 4, ///< the file could not be read, or an import went missing +}; + +struct Options { + std::filesystem::path source; + std::vector search_paths; + bool no_standard_library = false; + bool strict = false; ///< treat warnings as failures + bool quiet = false; +}; + +void print_usage() { + std::cout << R"(scena-check — check an OpenSCENARIO DSL file + +usage: scena-check [options] + + an OpenSCENARIO DSL file (.osc) + +options: + -I, --search-path directory to resolve module imports against; + repeatable, searched in the order given + --no-standard-library do not load the bundled osc.standard library + --strict exit non-zero when there are warnings, not just + errors + --quiet do not print diagnostics to stderr + -h, --help this text + +exit codes: + 0 ok 2 usage 3 the source did not check 4 the input could not be read + +scena-check does not execute the scenario; it loads it, follows its imports and +reports what the checker finds. +)"; +} + +const char* severity_name(scena::Severity severity) { + switch (severity) { + case scena::Severity::Info: + return "info"; + case scena::Severity::Warning: + return "warning"; + case scena::Severity::Error: + break; + } + return "error"; +} + +/// Same one-diagnostic-per-line shape scena-run prints, so a script that +/// already parses one tool's output can parse the other's. +void print_diagnostics(const std::vector& diagnostics, bool quiet) { + if (quiet) { + return; + } + for (const scena::Diagnostic& diagnostic : diagnostics) { + std::cerr << severity_name(diagnostic.severity) << ": "; + if (!diagnostic.location.file.empty()) { + std::cerr << diagnostic.location.file; + if (diagnostic.location.line > 0) { + std::cerr << ':' << diagnostic.location.line << ':' << diagnostic.location.column; + } + std::cerr << ": "; + } + if (!diagnostic.path.empty()) { + std::cerr << diagnostic.path << ": "; + } + std::cerr << diagnostic.message; + // The DSL standard defines no `asam.net:` rule identifiers, so a DSL + // diagnostic cites a section in its message and leaves this empty. The + // branch is here because the same Diagnostic type carries XML rule ids. + if (!diagnostic.rule_id.empty()) { + std::cerr << " [" << diagnostic.rule_id << ']'; + } + std::cerr << '\n'; + } +} + +bool parse_options(int argc, char** argv, Options& options, int& exit_code) { + for (int index = 1; index < argc; ++index) { + const std::string_view argument = argv[index]; + if (argument == "-h" || argument == "--help") { + print_usage(); + exit_code = kOk; + return false; + } + if (argument == "-I" || argument == "--search-path") { + if (index + 1 >= argc) { + std::cerr << "error: " << argument << " needs a directory\n"; + exit_code = kUsage; + return false; + } + options.search_paths.emplace_back(argv[++index]); + continue; + } + if (argument == "--no-standard-library") { + options.no_standard_library = true; + continue; + } + if (argument == "--strict") { + options.strict = true; + continue; + } + if (argument == "--quiet") { + options.quiet = true; + continue; + } + if (!argument.empty() && argument.front() == '-') { + std::cerr << "error: unknown option '" << argument << "'\n"; + exit_code = kUsage; + return false; + } + if (!options.source.empty()) { + std::cerr << "error: more than one source file given\n"; + exit_code = kUsage; + return false; + } + options.source = argument; + } + if (options.source.empty()) { + print_usage(); + exit_code = kUsage; + return false; + } + return true; +} + +} // namespace + +int main(int argc, char** argv) { + Options options; + int exit_code = kOk; + if (!parse_options(argc, argv, options, exit_code)) { + return exit_code; + } + + scena::dsl::LoadOptions load_options; + load_options.search_paths = options.search_paths; + load_options.implicit_standard_library = !options.no_standard_library; + + scena::dsl::LoadResult loaded; + scena::dsl::Program program; + scena::DiagnosticSink sink; + const scena::Status status = + scena::dsl::check_file(options.source, load_options, loaded, program, sink); + + print_diagnostics(sink.diagnostics(), options.quiet); + + // `InvalidArgument` is the Status model's host-misuse code: the path was + // not something we could read at all. Everything else that fails is a + // defect in the content, which is the other kind of exit. + if (status == scena::Status::InvalidArgument) { + return kInputFailed; + } + if (status != scena::Status::Ok) { + return kCheckFailed; + } + if (options.strict) { + for (const scena::Diagnostic& diagnostic : sink.diagnostics()) { + if (diagnostic.severity == scena::Severity::Warning) { + return kCheckFailed; + } + } + } + if (!options.quiet) { + std::cout << options.source.string() << ": ok, " << program.types.size() << " types across " + << loaded.files().size() << " files\n"; + } + return kOk; +} diff --git a/tools/scena-check/tests/CMakeLists.txt b/tools/scena-check/tests/CMakeLists.txt new file mode 100644 index 0000000..81fd263 --- /dev/null +++ b/tools/scena-check/tests/CMakeLists.txt @@ -0,0 +1,18 @@ +# SPDX-FileCopyrightText: 2026 Robomous +# SPDX-License-Identifier: Apache-2.0 +include(GoogleTest) + +# The CLI's behaviour is what a script depends on — the exit codes and the +# shape of a diagnostic line — so the test drives the built binary as a +# subprocess rather than linking its internals. +add_executable(scena_check_test scena_check_test.cpp) +target_link_libraries(scena_check_test PRIVATE GTest::gtest_main) +target_compile_features(scena_check_test PRIVATE cxx_std_20) +set_target_properties(scena_check_test PROPERTIES CXX_EXTENSIONS OFF) +target_compile_definitions(scena_check_test + PRIVATE SCENA_CHECK_BINARY="$") +scn_set_warnings(scena_check_test) +scn_set_fp_strictness(scena_check_test) +scn_enable_sanitizers(scena_check_test) +add_dependencies(scena_check_test scena-check) +gtest_discover_tests(scena_check_test DISCOVERY_TIMEOUT 60) diff --git a/tools/scena-check/tests/scena_check_test.cpp b/tools/scena-check/tests/scena_check_test.cpp new file mode 100644 index 0000000..87f4258 --- /dev/null +++ b/tools/scena-check/tests/scena_check_test.cpp @@ -0,0 +1,308 @@ +/* + * Copyright 2026 Robomous + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// +// scena-check's contract is what a script depends on — the exit code and the +// shape of a diagnostic line — so these drive the built binary as a subprocess +// rather than linking its internals (p7-s5, #43). +// + +#include +#include +#include +#include +#include +#include + +#include + +namespace { + +namespace fs = std::filesystem; + +/// Runs scena-check with `args`, returning its exit code. Output goes to a file +/// so a failing test can show what the tool said. +int run(const std::string& args, std::string& output) { + const fs::path log = fs::temp_directory_path() / "scena_check_test.log"; + // Quoting the binary path: a build directory may contain spaces. + std::string command = + std::string("\"") + SCENA_CHECK_BINARY + "\" " + args + " > \"" + log.string() + "\" 2>&1"; +#ifdef _WIN32 + // cmd.exe strips the outer pair of quotes from the command it is given, so + // a command that *starts* with a quoted path loses it. Wrapping the whole + // thing in one more pair is the documented way to keep both. + command = "\"" + command + "\""; +#endif + const int status = std::system(command.c_str()); + std::ifstream file(log); + std::stringstream buffer; + buffer << file.rdbuf(); + output = buffer.str(); +#ifdef _WIN32 + return status; +#else + // POSIX packs the exit status; WEXITSTATUS without . + return (status & 0x7f) == 0 ? ((status >> 8) & 0xff) : -1; +#endif +} + +/// A self-cleaning temporary directory to write .osc files into. +class Tree { +public: + Tree() { + root_ = fs::temp_directory_path() / + ("scena_check_" + std::to_string(reinterpret_cast(this))); + std::error_code ignored; + fs::remove_all(root_, ignored); + fs::create_directories(root_); + } + ~Tree() { + std::error_code ignored; + fs::remove_all(root_, ignored); + } + Tree(const Tree&) = delete; + Tree& operator=(const Tree&) = delete; + + fs::path write(const std::string& name, const std::string& contents) const { + const fs::path path = root_ / name; + fs::create_directories(path.parent_path()); + std::ofstream file(path, std::ios::binary); + file << contents; + return path; + } + + [[nodiscard]] const fs::path& root() const { return root_; } + +private: + fs::path root_; +}; + +/// Wraps a path for the command line; a temp directory may contain spaces. +std::string quoted(const fs::path& path) { + return "\"" + path.string() + "\""; +} + +constexpr int kOk = 0; +constexpr int kUsage = 2; +constexpr int kCheckFailed = 3; +constexpr int kInputFailed = 4; + +TEST(ScenaCheckTest, AValidFileChecksClean) { + const Tree tree; + const fs::path source = tree.write("ok.osc", "import osc.standard.all\n" + "namespace demo use std, stdtypes\n" + "scenario cut_in:\n" + " ego: vehicle\n" + " target: lane\n" + " keep(target.lane_type == lane_type!driving)\n" + " keep(target.width == 3.5m)\n"); + std::string output; + EXPECT_EQ(run(quoted(source), output), kOk) << output; + EXPECT_NE(output.find("ok,"), std::string::npos) << output; +} + +TEST(ScenaCheckTest, TheStandardLibraryIsAvailableWithoutAnImport) { + // §8.14's physical types are what give `30kph` a type at all, so the types + // sub-module is loaded for every check (ADR-0029). + const Tree tree; + const fs::path source = tree.write("implicit.osc", "struct probe:\n" + " v: speed\n" + " keep(it.v == 30kph)\n"); + std::string output; + EXPECT_EQ(run(quoted(source), output), kOk) << output; +} + +TEST(ScenaCheckTest, NoStandardLibrarySuppressesIt) { + const Tree tree; + const fs::path source = tree.write("implicit.osc", "struct probe:\n" + " v: speed\n"); + std::string output; + EXPECT_EQ(run(quoted(source) + " --no-standard-library", output), kCheckFailed) << output; + EXPECT_NE(output.find("speed"), std::string::npos) << output; +} + +TEST(ScenaCheckTest, AnErrorExitsThreeAndCitesItsSection) { + // DSL diagnostics cite a § in the message: the DSL standard defines no + // `asam.net:` rule identifiers, so there is no rule id to print. + const Tree tree; + const fs::path source = tree.write("bad.osc", "struct probe:\n" + " v: no_such_type\n"); + std::string output; + EXPECT_EQ(run(quoted(source), output), kCheckFailed) << output; + EXPECT_NE(output.find("error: "), std::string::npos) << output; + EXPECT_NE(output.find('\xc2'), std::string::npos) << "expected a § reference: " << output; + EXPECT_EQ(output.find('['), std::string::npos) << "no rule id belongs here: " << output; +} + +TEST(ScenaCheckTest, ADiagnosticNamesTheFileAndPosition) { + const Tree tree; + const fs::path source = tree.write("located.osc", "struct probe:\n" + " v: no_such_type\n"); + std::string output; + EXPECT_EQ(run(quoted(source), output), kCheckFailed) << output; + EXPECT_NE(output.find("located.osc:2:"), std::string::npos) << output; +} + +TEST(ScenaCheckTest, AMissingFileExitsFourNotThree) { + // The Status model draws the line between a defect in the content and the + // host handing us something unusable; the exit codes follow it. + const Tree tree; + std::string output; + EXPECT_EQ(run(quoted(tree.root() / "absent.osc"), output), kInputFailed) << output; +} + +TEST(ScenaCheckTest, QuietPrintsNothingButStillExitsNonZero) { + const Tree tree; + const fs::path source = tree.write("bad.osc", "struct probe:\n" + " v: no_such_type\n"); + std::string output; + EXPECT_EQ(run(quoted(source) + " --quiet", output), kCheckFailed) << output; + EXPECT_TRUE(output.empty()) << output; +} + +TEST(ScenaCheckTest, StrictTurnsAWarningIntoAFailure) { + // §7.7.4 reserves the `std`-prefixed namespaces for the standard, so this + // warns. Without --strict a warning is still a clean exit. + const Tree tree; + const fs::path source = tree.write("warn.osc", "namespace stdthing\n" + "struct probe:\n" + " v: int\n"); + std::string output; + EXPECT_EQ(run(quoted(source), output), kOk) << output; + EXPECT_NE(output.find("warning: "), std::string::npos) << output; + + std::string strict_output; + EXPECT_EQ(run(quoted(source) + " --strict", strict_output), kCheckFailed) << strict_output; +} + +TEST(ScenaCheckTest, ASearchPathResolvesAModuleImport) { + // §7.7.5.1.2 maps `a.b.c` to `a/b/c.osc` under the configured search paths. + const Tree tree; + tree.write("lib/shared/types.osc", "struct helper:\n" + " v: int\n" + "export *\n"); + const fs::path source = tree.write("main.osc", "import shared.types\n" + "struct probe:\n" + " h: helper\n"); + std::string output; + EXPECT_EQ(run(quoted(source), output), kCheckFailed) + << "without -I the module must not resolve: " << output; + + std::string found; + EXPECT_EQ(run(quoted(source) + " -I " + quoted(tree.root() / "lib"), found), kOk) << found; +} + +TEST(ScenaCheckTest, SearchPathsAreTriedInTheOrderGiven) { + const Tree tree; + tree.write("first/shared/types.osc", "struct helper:\n" + " marker_first: int\n" + "export *\n"); + tree.write("second/shared/types.osc", "struct helper:\n" + " marker_second: int\n" + "export *\n"); + const fs::path source = tree.write("main.osc", "import shared.types\n" + "struct probe:\n" + " h: helper\n" + " keep(it.h.marker_first == 1)\n"); + std::string output; + EXPECT_EQ(run(quoted(source) + " -I " + quoted(tree.root() / "first") + " -I " + + quoted(tree.root() / "second"), + output), + kOk) + << output; + + std::string reversed; + EXPECT_EQ(run(quoted(source) + " -I " + quoted(tree.root() / "second") + " -I " + + quoted(tree.root() / "first"), + reversed), + kCheckFailed) + << "the second copy has no marker_first: " << reversed; +} + +TEST(ScenaCheckTest, AMissingImportExitsThree) { + // An unresolvable import is a defect in the content, not host misuse: the + // file we were handed is readable, it just names something that is not + // there. + const Tree tree; + const fs::path source = tree.write("main.osc", "import shared.absent\n"); + std::string output; + EXPECT_EQ(run(quoted(source), output), kCheckFailed) << output; +} + +TEST(ScenaCheckTest, HelpExitsZeroAndDescribesTheExitCodes) { + std::string output; + EXPECT_EQ(run("--help", output), kOk) << output; + EXPECT_NE(output.find("exit codes:"), std::string::npos) << output; + EXPECT_NE(output.find("--search-path"), std::string::npos) << output; +} + +TEST(ScenaCheckTest, NoArgumentsExitsUsage) { + std::string output; + EXPECT_EQ(run("", output), kUsage) << output; + EXPECT_NE(output.find("usage:"), std::string::npos) << output; +} + +TEST(ScenaCheckTest, AnUnknownOptionExitsUsage) { + const Tree tree; + const fs::path source = tree.write("ok.osc", "struct probe:\n" + " v: int\n"); + std::string output; + EXPECT_EQ(run(quoted(source) + " --nonsense", output), kUsage) << output; +} + +TEST(ScenaCheckTest, TwoSourceFilesExitUsage) { + const Tree tree; + const fs::path first = tree.write("a.osc", "struct a:\n v: int\n"); + const fs::path second = tree.write("b.osc", "struct b:\n v: int\n"); + std::string output; + EXPECT_EQ(run(quoted(first) + " " + quoted(second), output), kUsage) << output; +} + +TEST(ScenaCheckTest, ASearchPathWithoutADirectoryExitsUsage) { + const Tree tree; + const fs::path source = tree.write("ok.osc", "struct probe:\n v: int\n"); + std::string output; + EXPECT_EQ(run(quoted(source) + " -I", output), kUsage) << output; +} + +TEST(ScenaCheckTest, EveryDiagnosticIsReportedNotJustTheFirst) { + // Error recovery is contractual: a malformed member never costs its + // siblings, so one run reports many diagnostics. + const Tree tree; + const fs::path source = tree.write("many.osc", "struct probe:\n" + " a: no_such_type\n" + " b: another_missing_type\n"); + std::string output; + EXPECT_EQ(run(quoted(source), output), kCheckFailed) << output; + EXPECT_NE(output.find("no_such_type"), std::string::npos) << output; + EXPECT_NE(output.find("another_missing_type"), std::string::npos) << output; +} + +TEST(ScenaCheckTest, CheckingIsDeterministic) { + // Load time is inside the determinism contract, and that includes the + // order diagnostics come out in. + const Tree tree; + const fs::path source = tree.write("many.osc", "struct probe:\n" + " a: no_such_type\n" + " b: another_missing_type\n"); + std::string first; + std::string second; + EXPECT_EQ(run(quoted(source), first), kCheckFailed); + EXPECT_EQ(run(quoted(source), second), kCheckFailed); + EXPECT_EQ(first, second); +} + +} // namespace