diff --git a/changes/unreleased/asciidoc-document-bodies.added.md b/changes/unreleased/asciidoc-document-bodies.added.md index 745af56906..2d9d5f6540 100644 --- a/changes/unreleased/asciidoc-document-bodies.added.md +++ b/changes/unreleased/asciidoc-document-bodies.added.md @@ -1 +1 @@ -- Document prose can be written as AsciiDoc: a KerML textual representation `rep language "asciidoc" /* … */` inside a `Document` or `Section` expands in place into content blocks, and one inside a `part : Paragraph` yields that paragraph, alongside the Markdown form. Paragraphs, admonitions (a strong lead label), bullet and numbered lists, tables of text cells under a header row (static `Table` blocks with `cols` widths), `[stem]` blocks (`Formula`) and block images (`Image`) map onto the existing content kinds; `_emphasis_`, `*strong*`, `` `monospaced` ``, `stem:[…]`, `link:`, URLs, `<>` and the new `sysml:ref[]` inline macro map onto runs, the reference forms resolved as `Ref::target` is. The body is parsed by Asciidoctor.js run as an external converter — `scripts/asciidoc/asciidoctor-json.js`, provisioned by `scripts/download-doc-pdf-toolchain.sh` and named by `OPENSYSML_ASCIIDOCTOR` — and a missing or failing converter is a typed planning error (`document-plan-markup-tool-missing`, `document-plan-markup-tool-failed`) naming the script and the variable. Constructs no content block renders, `sysml:value[…]` among them, are refused with `document-plan-unsupported-markup-construct` naming the construct and its body line. +- Document prose can be written as AsciiDoc: a KerML textual representation `rep language "asciidoc" /* … */` inside a `Document` or `Section` expands in place into content blocks, and one inside a `part : Paragraph` yields that paragraph, alongside the Markdown form. Paragraphs, admonitions (a strong lead label), bullet and numbered lists, tables of text cells under a header row (static `Table` blocks with `cols` widths), `[stem]` blocks (`Formula`) and block images (`Image`) map onto the existing content kinds; `_emphasis_`, `*strong*`, `` `monospaced` ``, `stem:[…]`, `link:`, URLs, `<>` and the new `sysml:ref[]` inline macro map onto runs, the reference forms resolved as `Ref::target` is. The body is parsed by Asciidoctor.js run as an external converter — `scripts/asciidoc/asciidoctor-json.js`, provisioned by `scripts/download-doc-pdf-toolchain.sh` and named by `OPENSYSML_ASCIIDOCTOR` — and a failing converter is a typed planning error (`document-plan-markup-tool-failed`), a missing one a validation warning (`document-plan-markup-tool-missing`), each naming the script and the variable. Constructs no content block renders, `sysml:value[…]` among them, are refused with `document-plan-unsupported-markup-construct` naming the construct and its body line. diff --git a/changes/unreleased/markup-tool-missing-warning.fixed.md b/changes/unreleased/markup-tool-missing-warning.fixed.md new file mode 100644 index 0000000000..520e23baa0 --- /dev/null +++ b/changes/unreleased/markup-tool-missing-warning.fixed.md @@ -0,0 +1 @@ +- `sysml -validate` no longer fails over a model whose `rep language "asciidoc"` bodies need a converter the host lacks: a missing markup converter is now the warning `document-plan-markup-tool-missing` at the document's first such body, naming the converter looked for, whether `OPENSYSML_ASCIIDOCTOR` named it or it was the default looked up on `PATH`, and the remedy, and the rest of the document is still validated with those bodies unchecked (`-warnings-as-errors` makes it a failure). A converter that is found but fails stays the error `document-plan-markup-tool-failed`, and rendering a document still fails when its converter is missing, since the rendering needs the body. A document reported on its converter being missing or failing writes no interface record, so installing the converter is seen on the next run rather than hidden by a cached verdict. diff --git a/cmd/sysml/markup_tool_missing_test.go b/cmd/sysml/markup_tool_missing_test.go new file mode 100644 index 0000000000..fd3fca093f --- /dev/null +++ b/cmd/sysml/markup_tool_missing_test.go @@ -0,0 +1,87 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Open-MBEE/OpenSysML/internal/ir/docplan/markup" +) + +const ( + selfModelDir = "../../examples/self-model" + markupToolMissingCode = "document-plan-markup-tool-missing" + markupToolMissingAdvice = "the AsciiDoc converter asciidoctor-json was not found on PATH (" + markup.AsciiDocEnv + " is unset); run ./scripts/download-doc-pdf-toolchain.sh and point " + markup.AsciiDocEnv + " at build/doc-pdf/asciidoctor/asciidoctor-json.js" +) + +// hostEnv is the environment of a host whose AsciiDoc converter is converter +// — none when empty: the variable unset and nothing on PATH — with a record +// cache of its own, so no verdict of another host's run is held. +func hostEnv(t *testing.T, converter string) []string { + t.Helper() + env := []string{markup.AsciiDocEnv + "=" + converter, "XDG_CACHE_HOME=" + t.TempDir()} + if converter == "" { + env = append(env, "PATH="+t.TempDir()) + } + return env +} + +func selfModelFiles(t *testing.T) []string { + t.Helper() + var files []string + for _, pattern := range []string{"*.sysml", "asciidoc/*.sysml"} { + matches, err := filepath.Glob(filepath.Join(selfModelDir, pattern)) + if err != nil || len(matches) == 0 { + t.Fatalf("glob %s: %v, %d files", pattern, err, len(matches)) + } + files = append(files, matches...) + } + return files +} + +// TestValidateWithoutMarkupConverterWarns checks that a model whose prose +// needs a converter the host lacks still validates: one warning names the +// converter, where it was looked for and the remedy, and the exit status is +// 0 unless -warnings-as-errors asks otherwise; a rendering, which needs the +// body, fails. +func TestValidateWithoutMarkupConverterWarns(t *testing.T) { + binary := buildCLI(t) + + result := runSysml(t, binary, hostEnv(t, ""), "-validate", selfModelDir) + wantReport(t, result, 0, "warning["+markupToolMissingCode+"]", "carries 3 asciidoc bodies that were not checked", markupToolMissingAdvice, "no errors") + if got := strings.Count(result.output(), "warning["); got != 1 { + t.Errorf("%d warnings, want exactly one:\n%s", got, result.output()) + } + + result = runSysml(t, binary, hostEnv(t, ""), "-validate", "-warnings-as-errors", selfModelDir) + wantReport(t, result, 2, "warning["+markupToolMissingCode+"]", "-warnings-as-errors") + + result = runSysml(t, binary, hostEnv(t, ""), "-validate", "-format", "sarif", selfModelDir) + if result.status != 0 || !strings.Contains(result.stdout, `"level": "warning"`) || !strings.Contains(result.stdout, markupToolMissingCode) { + t.Errorf("-format sarif exited %d without the warning:\n%s", result.status, result.output()) + } + + args := append(selfModelFiles(t), "-render-document", "OpenSysMLAsciiDoc::MarkupNote") + result = runSysml(t, binary, hostEnv(t, ""), args...) + wantReport(t, result, 2, "carries a asciidoc body that could not be parsed: "+markupToolMissingAdvice) +} + +// TestValidateWithMarkupConverterChecksBodies runs with the converter the +// environment names, as the PDF toolchain job does, and wants the bodies +// checked: no warning. Without one it skips, unless the job requires it. +func TestValidateWithMarkupConverterChecksBodies(t *testing.T) { + converter := os.Getenv(markup.AsciiDocEnv) + if converter == "" { + if required := os.Getenv("OPENSYSML_REQUIRE_PDF_TOOLCHAIN"); required != "" { + t.Fatalf("OPENSYSML_REQUIRE_PDF_TOOLCHAIN=%s but %s is unset", required, markup.AsciiDocEnv) + } + t.Skipf("%s is unset", markup.AsciiDocEnv) + } + binary := buildCLI(t) + result := runSysml(t, binary, hostEnv(t, converter), "-validate", "-warnings-as-errors", selfModelDir) + wantReport(t, result, 0, "no errors") + if strings.Contains(result.output(), markupToolMissingCode) { + t.Errorf("the converter %s was found, yet a body went unchecked:\n%s", converter, result.output()) + } +} diff --git a/docs/internals/interface-records.md b/docs/internals/interface-records.md index e98ed8aa6a..d732d7129b 100644 --- a/docs/internals/interface-records.md +++ b/docs/internals/interface-records.md @@ -155,6 +155,15 @@ whose `baseType` is bound conditionally. In the four OMG corpora one document of (`kerml-examples/Simple Tests/MetadataTest.kerml`, a conditional `baseType` binding). A refusal is never silent: the error names the fact and the element. +The workspace also refuses what a document plan in another document reads of +this one and no record answers (`plannedElsewhereLocked`): a document query +definition, whose result expression the plan evaluates from the tree, and a +`rep language "asciidoc"` body, which an external converter checks, so that +its verdict — the body checked, or the converter missing +(`document-plan-markup-tool-missing`) or failing — is the host's and would not +hold on another. Such a document is analyzed again wherever it is next +opened, and installing the converter is seen at once. + ## Cache key `Cache.InterfaceKey(name, content, libraryIdentity, mode)` is the document's diff --git a/docs/manual/authoring.md b/docs/manual/authoring.md index 20b90a6767..1f6da13ed1 100644 --- a/docs/manual/authoring.md +++ b/docs/manual/authoring.md @@ -404,9 +404,16 @@ the content blocks of this chapter. `./scripts/download-doc-pdf-toolchain.sh` installs the pinned Asciidoctor.js beside the script under `build/doc-pdf/asciidoctor/`; point `OPENSYSML_ASCIIDOCTOR` at the installed `asciidoctor-json.js` (an `asciidoctor-json` on `PATH` is found without it). -A document with an AsciiDoc body and no converter fails to plan with a typed -error naming the script and the variable, and a converter that fails is -reported with its message — the body is never rendered as literal text. +Validation (`sysml -validate`, the editors, the SARIF report) does not need +the converter installed: a document with an AsciiDoc body and no converter +gets one `document-plan-markup-tool-missing` warning at its first body, naming +the converter looked for, where (`OPENSYSML_ASCIIDOCTOR` or `PATH`) and the +remedy, and its AsciiDoc bodies go unchecked while the rest of the document +is still validated; `-warnings-as-errors` makes that a failure. A rendering +needs the body parsed, so `-render-document` fails with the same message +until the converter is installed, and a converter that is found but fails is +an error reported with its message — the body is never rendered as literal +text. | AsciiDoc | Content | | --- | --- | diff --git a/docs/reference/diagnostics.md b/docs/reference/diagnostics.md index b515293064..3cbda0e026 100644 --- a/docs/reference/diagnostics.md +++ b/docs/reference/diagnostics.md @@ -2145,7 +2145,7 @@ Specification: Model-defined rule; the predicate is a KerML constraint evaluated A document definition (one typed by `DocumentDefinitions::Document` or a specialization of it) is compiled into a document plan before it is rendered; the member code names the rule that failed — a content with no title, an attribute that is not one string literal, a nested document, a query the document does not declare, a diagram kind no rendering supports — and the message states where. Correct the member the message names; the plain `document-plan` code reports a failure no rule classifies, with the planner's own message. -Emitted as `document-plan-`, one code per rule: `document-plan-invalid-context`, `document-plan-library-unavailable`, `document-plan-not-document-definition`, `document-plan-missing-title`, `document-plan-invalid-attribute`, `document-plan-invalid-content`, `document-plan-nested-document`, `document-plan-missing-text`, `document-plan-conflicting-text`, `document-plan-missing-query`, `document-plan-conflicting-query`, `document-plan-unknown-query`, `document-plan-query-planning`, `document-plan-invalid-style`, `document-plan-unknown-parameter`, `document-plan-duplicate-binding`, `document-plan-missing-binding`, `document-plan-unsupported-binding`, `document-plan-binding-type`, `document-plan-binding-multiplicity`, `document-plan-missing-view-source`, `document-plan-invalid-view-source`, `document-plan-unknown-view-source`, `document-plan-missing-diagram-kind`, `document-plan-conflicting-diagram-kind`, `document-plan-unsupported-diagram-kind`, `document-plan-invalid-direction`, `document-plan-unsupported-direction`, `document-plan-invalid-palette`, `document-plan-unsupported-palette`, `document-plan-invalid-ports`, `document-plan-unsupported-ports`, `document-plan-invalid-show`, `document-plan-invalid-overlay`, `document-plan-unsupported-overlay`, `document-plan-conflicting-runs`, `document-plan-ambiguous-run`, `document-plan-missing-run-text`, `document-plan-invalid-run-style`, `document-plan-missing-link-target`, `document-plan-missing-ref-target`, `document-plan-unknown-ref-target`, `document-plan-invalid-ref-target`, `document-plan-ambiguous-ref-target`, `document-plan-unknown-value-target`, `document-plan-invalid-value-target`, `document-plan-unknown-group-column`, `document-plan-invalid-column-widths`, `document-plan-invalid-column-labels`, `document-plan-column-run-without-query`, `document-plan-conflicting-column-runs`, `document-plan-missing-run-column`, `document-plan-conflicting-run-style`, `document-plan-unknown-run-column`, `document-plan-missing-definition-column`, `document-plan-unknown-definition-column`, `document-plan-missing-formula-source`, `document-plan-missing-image-location`, `document-plan-unsupported-markup-language`, `document-plan-unsupported-markup-construct`, `document-plan-misplaced-markup-body`, `document-plan-markup-body-not-paragraph`, `document-plan-markup-tool-missing`, `document-plan-markup-tool-failed`, `document-plan-markup-heading-level`. +Emitted as `document-plan-`, one code per rule: `document-plan-invalid-context`, `document-plan-library-unavailable`, `document-plan-not-document-definition`, `document-plan-missing-title`, `document-plan-invalid-attribute`, `document-plan-invalid-content`, `document-plan-nested-document`, `document-plan-missing-text`, `document-plan-conflicting-text`, `document-plan-missing-query`, `document-plan-conflicting-query`, `document-plan-unknown-query`, `document-plan-query-planning`, `document-plan-invalid-style`, `document-plan-unknown-parameter`, `document-plan-duplicate-binding`, `document-plan-missing-binding`, `document-plan-unsupported-binding`, `document-plan-binding-type`, `document-plan-binding-multiplicity`, `document-plan-missing-view-source`, `document-plan-invalid-view-source`, `document-plan-unknown-view-source`, `document-plan-missing-diagram-kind`, `document-plan-conflicting-diagram-kind`, `document-plan-unsupported-diagram-kind`, `document-plan-invalid-direction`, `document-plan-unsupported-direction`, `document-plan-invalid-palette`, `document-plan-unsupported-palette`, `document-plan-invalid-ports`, `document-plan-unsupported-ports`, `document-plan-invalid-show`, `document-plan-invalid-overlay`, `document-plan-unsupported-overlay`, `document-plan-conflicting-runs`, `document-plan-ambiguous-run`, `document-plan-missing-run-text`, `document-plan-invalid-run-style`, `document-plan-missing-link-target`, `document-plan-missing-ref-target`, `document-plan-unknown-ref-target`, `document-plan-invalid-ref-target`, `document-plan-ambiguous-ref-target`, `document-plan-unknown-value-target`, `document-plan-invalid-value-target`, `document-plan-unknown-group-column`, `document-plan-invalid-column-widths`, `document-plan-invalid-column-labels`, `document-plan-column-run-without-query`, `document-plan-conflicting-column-runs`, `document-plan-missing-run-column`, `document-plan-conflicting-run-style`, `document-plan-unknown-run-column`, `document-plan-missing-definition-column`, `document-plan-unknown-definition-column`, `document-plan-missing-formula-source`, `document-plan-missing-image-location`, `document-plan-unsupported-markup-language`, `document-plan-unsupported-markup-construct`, `document-plan-misplaced-markup-body`, `document-plan-markup-body-not-paragraph`, `document-plan-markup-tool-missing` (a warning), `document-plan-markup-tool-failed`, `document-plan-markup-heading-level`. Specification: OpenSysML DocumentDefinitions library diff --git a/docs/reference/environment.md b/docs/reference/environment.md index 218b24a606..ad85566595 100644 --- a/docs/reference/environment.md +++ b/docs/reference/environment.md @@ -33,7 +33,7 @@ run that would never finish into a reported error instead of a hang. | `OPENSYSML_GRPC_MAX_HELD_OBJECTS` | `10000` | The most objects `sysml-grpc` keeps for one cached model, nested objects counted: `Instantiate` creates them and document queries bind and enumerate them for as long as the model stays cached. An `Instantiate`, query or render whose objects would pass the bound fails whole with `RESOURCE_EXHAUSTED`, leaving none of them, until the model leaves the cache, which releases them together; nothing is evicted behind an id a client holds. Read at startup | | `OPENSYSML_WEASYPRINT`, `OPENSYSML_PANDOC`, `OPENSYSML_PRINCE` | unset (look on `PATH`) | Executable of the HTML-to-PDF converter `-render-document -doc-form pdf` drives under `-pdf-engine weasyprint` (the default; `pandoc` also needs it), `pandoc` or `prince`; the selected one absent is a typed `tool-missing` error naming its variable | | `OPENSYSML_MMDC`, `OPENSYSML_MMDC_PUPPETEER` | unset (look for `mmdc` on `PATH`) | Mermaid CLI, which draws a PDF's Mermaid diagrams, and a Puppeteer configuration file for its browser (`--no-sandbox` in a container). Required as soon as a PDF has a Mermaid diagram; a document without one needs neither | -| `OPENSYSML_ASCIIDOCTOR` | unset (look for `asciidoctor-json` on `PATH`) | The AsciiDoc converter that parses `rep language "asciidoc"` prose bodies in generated documents: `scripts/asciidoc/asciidoctor-json.js` with Asciidoctor.js installed beside it, as `scripts/download-doc-pdf-toolchain.sh` provisions under `build/doc-pdf/asciidoctor/`. Without it, a document with an AsciiDoc body fails to plan naming the script and this variable. | +| `OPENSYSML_ASCIIDOCTOR` | unset (look for `asciidoctor-json` on `PATH`) | The AsciiDoc converter that parses `rep language "asciidoc"` prose bodies in generated documents: `scripts/asciidoc/asciidoctor-json.js` with Asciidoctor.js installed beside it, as `scripts/download-doc-pdf-toolchain.sh` provisions under `build/doc-pdf/asciidoctor/`. Without it, validating a document with an AsciiDoc body warns (`document-plan-markup-tool-missing`, a failure under `-warnings-as-errors`) that the bodies went unchecked, naming the converter, where it was looked for and this variable, and rendering the document fails with the same message. | | `OPENSYSML_KATEX`, `OPENSYSML_KATEX_CSS` | unset (look for `katex` on `PATH`, its stylesheet beside it) | KaTeX, which typesets a PDF's formulas, and its stylesheet when it is not installed beside the command. Required as soon as a PDF has a formula | | `OPENSYSML_DOT` | unset (look for `dot` on `PATH`) | Graphviz, which draws a PDF's diagrams under `-diagram-form dot` — `-Tsvg`, under the engine each block's `// layout:` header names (`dot`, `neato`, `neato -n`), so positioned views are drawn where the model put them. Optional: without it every DOT block stays in the PDF as source under a notice naming this variable | | `OPENSYSML_PLANTUML_JAR`, `OPENSYSML_JAVA` | unset; unset (look for `java` on `PATH`) | The PlantUML jar that draws a PDF's diagrams under `-diagram-form plantuml` (`java -jar -tsvg -pipe`) and the Java that runs it. Optional: without the jar or a Java every PlantUML block stays in the PDF as source under a notice naming the variable to set. `go test ./internal/ir/view` also passes its PlantUML goldens through the jar's `-checkonly` when the jar is named | diff --git a/examples/self-model/README.md b/examples/self-model/README.md index c35dee6368..2deab9e997 100644 --- a/examples/self-model/README.md +++ b/examples/self-model/README.md @@ -245,7 +245,8 @@ not. `-doc-form html` and `-doc-form asciidoc` render the same note as HTML and back to AsciiDoc. Without the converter the command fails to plan the document with an error naming -`OPENSYSML_ASCIIDOCTOR` and the download script, and nothing is rendered as literal text. The +`OPENSYSML_ASCIIDOCTOR` and the download script, and nothing is rendered as literal text; +`-validate` over the model instead warns once that the note's bodies went unchecked. The note's test, `TestSelfModelAsciiDocNoteRendersInstalled` in [`../self_model_asciidoc_test.go`](../self_model_asciidoc_test.go), renders all three forms and pins every value run against the model; it skips with the same instruction when the converter is diff --git a/internal/check/codes/catalogue.go b/internal/check/codes/catalogue.go index 73905c29d7..fddb5e613e 100644 --- a/internal/check/codes/catalogue.go +++ b/internal/check/codes/catalogue.go @@ -41,6 +41,9 @@ func Catalogue() string { b.WriteString(", ") } fmt.Fprintf(&b, "`%s-%s`", e.Code, k) + if severity := e.MemberSeverity(k); severity != e.Severity { + fmt.Fprintf(&b, " (a %s)", severity) + } } b.WriteString(".\n") } diff --git a/internal/check/codes/codes.go b/internal/check/codes/codes.go index 3c47b9876a..45990915ba 100644 --- a/internal/check/codes/codes.go +++ b/internal/check/codes/codes.go @@ -68,11 +68,23 @@ type Entry struct { // Kinds, for a family code, are the suffixes the emitted code appends to // Code after a dash, each naming one rule of the family. Kinds []string + // KindSeverities, for a family code, are the severities of the members + // that are not reported at the family's Severity, by kind. + KindSeverities map[string]string } // IsFamily reports whether the entry stands for a family of codes `Code-`. func (e Entry) IsFamily() bool { return len(e.Kinds) > 0 } +// MemberSeverity is the severity the member of the family named kind is +// reported with; the empty kind is the family's base code. +func (e Entry) MemberSeverity(kind string) string { + if severity, ok := e.KindSeverities[kind]; ok { + return severity + } + return e.Severity +} + // CatalogueURL is the published catalogue every code is a section of. const CatalogueURL = "https://runtime.opensysml.org/reference/diagnostics/" @@ -152,7 +164,7 @@ func Explain(code string) (string, error) { return "", fmt.Errorf("unknown diagnostic code %q; `sysml -explain` lists them", code) } var b strings.Builder - fmt.Fprintf(&b, "%s: %s (%s)\n", code, e.Severity, e.Tier) + fmt.Fprintf(&b, "%s: %s (%s)\n", code, e.MemberSeverity(kind), e.Tier) fmt.Fprintf(&b, " %s\n\n", e.Summary) b.WriteString(wrap(e.Meaning, 78)) if kind != "" { @@ -181,7 +193,7 @@ func Rows() []Row { out = append(out, Row{Code: e.Code, Severity: e.Severity, Tier: e.Tier, Summary: e.Summary}) // A family's base code is emitted too, for an error outside its kinds. for _, k := range e.Kinds { - out = append(out, Row{Code: e.Code + "-" + k, Severity: e.Severity, Tier: e.Tier, Summary: e.Summary + " (rule " + k + ")"}) + out = append(out, Row{Code: e.Code + "-" + k, Severity: e.MemberSeverity(k), Tier: e.Tier, Summary: e.Summary + " (rule " + k + ")"}) } } return out diff --git a/internal/check/codes/codes_test.go b/internal/check/codes/codes_test.go index 363d414ea4..6258f2ea3b 100644 --- a/internal/check/codes/codes_test.go +++ b/internal/check/codes/codes_test.go @@ -2,6 +2,7 @@ package codes import ( "regexp" + "slices" "strings" "testing" ) @@ -35,6 +36,31 @@ func TestEntriesAreWellFormed(t *testing.T) { t.Errorf("%s: kind %q is not kebab-case", e.Code, k) } } + for k, severity := range e.KindSeverities { + if !slices.Contains(e.Kinds, k) { + t.Errorf("%s: severity for %q, which is no kind of the family", e.Code, k) + } + if severity == e.Severity || (severity != "error" && severity != "warning") { + t.Errorf("%s-%s: severity %q is not the other of error and warning", e.Code, k, severity) + } + } + } +} + +func TestFamilyMembersCarryTheirOwnSeverity(t *testing.T) { + const missing, failed = "document-plan-markup-tool-missing", "document-plan-markup-tool-failed" + severities := make(map[string]string) + for _, row := range Rows() { + severities[row.Code] = row.Severity + } + if severities[missing] != "warning" || severities[failed] != "error" || severities["document-plan"] != "error" { + t.Errorf("Rows severities: %s=%q %s=%q document-plan=%q, want warning, error, error", missing, severities[missing], failed, severities[failed], severities["document-plan"]) + } + for code, want := range map[string]string{missing: missing + ": warning (document)", failed: failed + ": error (document)"} { + text, err := Explain(code) + if err != nil || !strings.HasPrefix(text, want) { + t.Errorf("Explain(%s) = %q, %v, want a text opening %q", code, text, err, want) + } } } diff --git a/internal/check/codes/table_document.go b/internal/check/codes/table_document.go index 429d4a22c4..7ce8be7799 100644 --- a/internal/check/codes/table_document.go +++ b/internal/check/codes/table_document.go @@ -3,6 +3,7 @@ package codes import ( "github.com/Open-MBEE/OpenSysML/internal/ir/docplan" "github.com/Open-MBEE/OpenSysML/internal/ir/queryplan" + "github.com/Open-MBEE/OpenSysML/internal/syntax/diag" ) // documentCodes are the codes of the document and query planning passes: two @@ -13,8 +14,9 @@ var documentCodes = []Entry{ Summary: "a document definition the document planner cannot compile", Meaning: "A document definition (one typed by `DocumentDefinitions::Document` or a specialization of it) is compiled into a document plan before it is rendered; the member code names the rule that failed — a content with no title, an attribute that is not one string literal, a nested document, a query the document does not declare, a diagram kind no rendering supports — and the message states where. " + "Correct the member the message names; the plain `document-plan` code reports a failure no rule classifies, with the planner's own message.", - Spec: "OpenSysML DocumentDefinitions library", - Kinds: docplanKinds(), + Spec: "OpenSysML DocumentDefinitions library", + Kinds: docplanKinds(), + KindSeverities: docplanKindSeverities(), }, { Code: "document-query", Tier: TierDocument, Severity: "error", @@ -35,6 +37,18 @@ func docplanKinds() []string { return kinds } +// docplanKindSeverities names the document planning error kinds reported as +// warnings, as docplan.ErrorKind.Severity decides. +func docplanKindSeverities() map[string]string { + severities := make(map[string]string) + for _, k := range docplan.ErrorKinds() { + if k.Severity() == diag.SeverityWarning { + severities[string(k)] = "warning" + } + } + return severities +} + // queryplanKinds lists the query planning error kinds, as queryplan.ErrorKind spells them. func queryplanKinds() []string { kinds := make([]string, 0, len(queryplan.ErrorKinds())) diff --git a/internal/check/passes/document/plan.go b/internal/check/passes/document/plan.go index ec337f7896..1068586f18 100644 --- a/internal/check/passes/document/plan.go +++ b/internal/check/passes/document/plan.go @@ -38,22 +38,32 @@ func (PlanPass) Run(ctx *kit.Context, name string, root *ast.RootNamespace) []di if ctx.DownstreamOfFailure(sym.Decl) { return } - if _, err := docplan.Compile(ctx.Index, ctx.Model(), ctx.Resolver(), sym); err != nil { - var planning *docplan.Error - if errors.As(err, &planning) { - if planning.Origin.Doc != "" && planning.Origin.Doc != name { - return - } - if ctx.DownstreamSpan(planning.Origin.Span) { - return - } + warnings, err := docplan.Validate(ctx.Index, ctx.Model(), ctx.Resolver(), sym) + for _, warning := range warnings { + if reportedHere(ctx, name, warning) { + diagnostics = append(diagnostics, planDiagnostic(warning)) } + } + if err != nil && reportedHere(ctx, name, err) { diagnostics = append(diagnostics, planDiagnostic(err)) } }) return diagnostics } +// reportedHere is whether a planning error is this document's to report: one +// placed in another document, or downstream of a failure, is not. +func reportedHere(ctx *kit.Context, name string, err error) bool { + var planning *docplan.Error + if !errors.As(err, &planning) { + return true + } + if planning.Origin.Doc != "" && planning.Origin.Doc != name { + return false + } + return !ctx.DownstreamSpan(planning.Origin.Span) +} + func planDiagnostic(err error) diag.Diagnostic { var planning *docplan.Error if !errors.As(err, &planning) { @@ -65,7 +75,7 @@ func planDiagnostic(err error) diag.Diagnostic { } } return diag.Diagnostic{ - Severity: diag.SeverityError, + Severity: planning.Kind.Severity(), Span: planning.Origin.Span, Message: planning.Error(), Code: planSource + "-" + string(planning.Kind), diff --git a/internal/check/passes/document/plan_test.go b/internal/check/passes/document/plan_test.go index 9aeba57ede..e4e8d1a181 100644 --- a/internal/check/passes/document/plan_test.go +++ b/internal/check/passes/document/plan_test.go @@ -1,11 +1,13 @@ package document_test import ( + "path/filepath" "strings" "testing" "github.com/Open-MBEE/OpenSysML/internal/check/passes" "github.com/Open-MBEE/OpenSysML/internal/check/passes/document" + "github.com/Open-MBEE/OpenSysML/internal/ir/docplan/markup" "github.com/Open-MBEE/OpenSysML/internal/syntax/diag" "github.com/Open-MBEE/OpenSysML/internal/syntax/parser" "github.com/Open-MBEE/OpenSysML/internal/syntax/source" @@ -143,3 +145,55 @@ part def Report :> Document { t.Fatalf("missing unknown-parameter diagnostic: %v", diagnostics) } } + +// documentPlanDiagnosticsWithSource analyses body as a batch does, with the +// notation readable, since a markup body is read from the source text. +func documentPlanDiagnosticsWithSource(t *testing.T, body string) []diag.Diagnostic { + t.Helper() + index := libs.NewModelIndex() + name := "documents.sysml" + sf := source.New(name, []byte(` +package Fixture { + private import DocumentQueries::*; + `+body+` +}`)) + p := parser.New(sf) + root := p.ParseFile() + index.AddDocument(name, root) + index.ExpandWildcardImports() + batch := &passes.Batch{Documents: []string{name}, Source: func(doc string, span source.Span) string { + if doc != name { + return "" + } + return sf.Text(span) + }} + passes.PrepareBatch(index, batch) + diagnostics, _ := passes.AnalyzeInBatch(name, source.KindSysML, root, parserDiagnostics(p), index, passes.Options{}, batch) + return diagnostics +} + +func TestDocumentPlanPassWarnsOfMissingMarkupConverterAndGoesOn(t *testing.T) { + t.Setenv(markup.AsciiDocEnv, filepath.Join(t.TempDir(), "missing-converter")) + diagnostics := documentPlanDiagnosticsWithSource(t, ` +part def Report :> Document { + attribute redefines title = "Report"; + part intro : Paragraph { + rep language "asciidoc" /* Overview. */ + } + part empty : Paragraph; +} +`) + if len(diagnostics) != 2 { + t.Fatalf("diagnostics = %v, want the converter warning and the missing text", diagnostics) + } + warning, missing := diagnostics[0], diagnostics[1] + if warning.Severity != diag.SeverityWarning || warning.Code != "document-plan-markup-tool-missing" || warning.Source != "document-plan" { + t.Errorf("warning = %+v, want a document-plan-markup-tool-missing warning", warning) + } + if !strings.Contains(warning.Message, "was not checked") || !strings.Contains(warning.Message, markup.AsciiDocEnv+" names it") || !strings.Contains(warning.Message, "download-doc-pdf-toolchain.sh") { + t.Errorf("warning message %q lacks the unchecked body, the variable or the remedy", warning.Message) + } + if missing.Severity != diag.SeverityError || missing.Code != "document-plan-missing-text" { + t.Errorf("error = %+v, want document-plan-missing-text", missing) + } +} diff --git a/internal/ir/docplan/asciidoc_bodies_test.go b/internal/ir/docplan/asciidoc_bodies_test.go index d79b1009c5..e314c0146e 100644 --- a/internal/ir/docplan/asciidoc_bodies_test.go +++ b/internal/ir/docplan/asciidoc_bodies_test.go @@ -8,6 +8,7 @@ import ( "github.com/Open-MBEE/OpenSysML/internal/ir/docplan/markup" "github.com/Open-MBEE/OpenSysML/internal/ir/docplan/markup/markuptest" + "github.com/Open-MBEE/OpenSysML/internal/syntax/diag" ) const asciiDocRecordings = "markup/testdata" @@ -140,3 +141,101 @@ func TestCompileAsciiDocBodyToolErrors(t *testing.T) { t.Errorf("kind %s: %v, want %s naming the converter's message", planning.Kind, err, ErrorMarkupToolFailed) } } + +func TestValidateLeavesBodiesOfMissingConverterUnchecked(t *testing.T) { + fixture := loadPlanningFixture(t, ` + part def Report :> Document { + attribute redefines title = "Report"; + rep language "AsciiDoc" /* A body. */ + part intro : Paragraph { + rep language "AsciiDoc" /* Another body. */ + } + part body : Section { + attribute redefines title = "Body"; + rep language "AsciiDoc" /* A third body. */ + } + part empty : Paragraph; + } + `) + t.Setenv(markup.AsciiDocEnv, filepath.Join(t.TempDir(), "missing-converter")) + + // Validation goes on past the bodies, one warning for all three, and + // still finds the paragraph that has no text. + warnings, err := Validate(fixture.index, fixture.model, fixture.resolver, fixture.symbol(t, "Report")) + if planning := planningError(t, err); planning.Kind != ErrorMissingText || planning.Content != "Observatory::Report::empty" { + t.Errorf("error %v, want %s at Observatory::Report::empty", err, ErrorMissingText) + } + if len(warnings) != 1 { + t.Fatalf("warnings %v, want one", warnings) + } + warning := warnings[0] + if warning.Kind != ErrorMarkupToolMissing || warning.Kind.Severity() != diag.SeverityWarning || warning.Bodies != 3 || warning.Content != "Observatory::Report" { + t.Errorf("warning %+v, want %s (a warning) over 3 bodies, the first in Observatory::Report", warning, ErrorMarkupToolMissing) + } + for _, want := range []string{"carries 3 asciidoc bodies that were not checked, the first in content Observatory::Report", "missing-converter", "(" + markup.AsciiDocEnv + " names it)", "download-doc-pdf-toolchain.sh"} { + if !strings.Contains(warning.Error(), want) { + t.Errorf("%v lacks %q", warning, want) + } + } + if ErrorMarkupToolFailed.Severity() != diag.SeverityError { + t.Errorf("%s is a %s, want an error", ErrorMarkupToolFailed, ErrorMarkupToolFailed.Severity()) + } + + // A rendering needs the bodies: Compile stops at the first. + _, err = fixture.compile(t, "Report") + if planning := bodyPlanningError(t, err); planning.Kind != ErrorMarkupToolMissing || planning.Bodies != 0 || !strings.Contains(err.Error(), "could not be parsed") { + t.Errorf("Compile: %v, want %s stopping at the body", err, ErrorMarkupToolMissing) + } +} + +func TestValidateReportsOneWarningPerBody(t *testing.T) { + fixture := loadPlanningFixture(t, ` + part def Report :> Document { + attribute redefines title = "Report"; + part intro : Paragraph { + rep language "AsciiDoc" /* A body. */ + } + } + `) + t.Setenv(markup.AsciiDocEnv, filepath.Join(t.TempDir(), "missing-converter")) + warnings, err := Validate(fixture.index, fixture.model, fixture.resolver, fixture.symbol(t, "Report")) + if err != nil { + t.Fatalf("Validate: %v", err) + } + if len(warnings) != 1 || warnings[0].Bodies != 1 || !strings.Contains(warnings[0].Error(), "content Observatory::Report::intro carries a asciidoc body that was not checked") { + t.Errorf("warnings %v, want one over the paragraph's body", warnings) + } +} + +// A document whose plan reads another document's outline leaves that +// document's unchecked bodies to its own plan: each warns once, of its own. +func TestValidateWarnsOfItsOwnBodiesOnly(t *testing.T) { + fixture := loadPlanningFixture(t, ` + part def Appendix :> Document { + attribute redefines title = "Appendix"; + rep language "AsciiDoc" /* An appendix body. */ + part tables : Section { + attribute redefines title = "Tables"; + rep language "markdown" /* ## Detail */ + } + } + part def Report :> Document { + attribute redefines title = "Report"; + rep language "markdown" /* See [](sysml:Appendix::tables.detail). */ + part note : Paragraph { + rep language "AsciiDoc" /* A report body. */ + } + } + `) + t.Setenv(markup.AsciiDocEnv, filepath.Join(t.TempDir(), "missing-converter")) + for _, name := range []string{"Report", "Appendix"} { + warnings, err := Validate(fixture.index, fixture.model, fixture.resolver, fixture.symbol(t, name)) + if err != nil { + t.Fatalf("Validate %s: %v", name, err) + } + fqn := "Observatory::" + name + if len(warnings) != 1 || warnings[0].Document != fqn || warnings[0].Bodies != 1 || !strings.HasPrefix(warnings[0].Content, fqn) { + t.Errorf("Validate %s: warnings %v, want one over its own body", name, warnings) + } + } +} diff --git a/internal/ir/docplan/compiler.go b/internal/ir/docplan/compiler.go index 270114ff17..d0c0badc2d 100644 --- a/internal/ir/docplan/compiler.go +++ b/internal/ir/docplan/compiler.go @@ -52,6 +52,17 @@ type compiler struct { targets map[*symbols.Symbol]refTarget parsed map[*symbols.Symbol]*parsedBody outlines map[*symbols.Symbol]*markupOutline + // lenient leaves a body whose converter is missing unchecked, warning once per + // language and scanned document (entry, or another whose outline is read). + lenient bool + scanned *symbols.Symbol + unchecked []*Error + uncheckedBy map[uncheckedKey]*Error +} + +type uncheckedKey struct { + document *symbols.Symbol + language string } type bases struct { @@ -91,6 +102,28 @@ func IsDocumentDefinition(index *symbols.Index, model *semantics.Model, sym *sym sym.Kind == symbols.SymbolPartDef && model != nil && model.Conforms(sym, base) } +// ReadsBody reports whether a document plan reads the body of the textual +// representation rep: one owned, at any depth, by a document, section or paragraph. +func ReadsBody(index *symbols.Index, model *semantics.Model, rep *symbols.Symbol) bool { + if rep == nil || rep.Kind != symbols.SymbolTextualRepresentation || model == nil { + return false + } + var bases []*symbols.Symbol + for _, fqn := range []string{documentBaseFQN, sectionBaseFQN, paragraphBaseFQN} { + if base := libraryBase(index, fqn); base != nil { + bases = append(bases, base) + } + } + for owner := rep.Owner(); owner != nil; owner = owner.Owner() { + for _, base := range bases { + if owner != base && model.Conforms(owner, base) { + return true + } + } + } + return false +} + // QueryTarget resolves the query definition a calc usage is typed by, following // redefinition lineage; nil when it is not typed by a query definition. func QueryTarget(index *symbols.Index, model *semantics.Model, resolver *resolve.Resolver, usage *symbols.Symbol) *symbols.Symbol { @@ -107,8 +140,22 @@ func QueryTarget(index *symbols.Index, model *semantics.Model, resolver *resolve // Compile compiles a document definition into an immutable document plan. func Compile(index *symbols.Index, model *semantics.Model, resolver *resolve.Resolver, entry *symbols.Symbol) (*Plan, error) { + plan, _, err := compile(index, model, resolver, entry, false) + return plan, err +} + +// Validate plans entry as Compile does, except that a markup body whose +// converter is missing is left unchecked and reported as a warning, one per +// language, so the rest of the document is still checked. A rendering needs +// every body parsed and uses Compile, which stops at such a body. +func Validate(index *symbols.Index, model *semantics.Model, resolver *resolve.Resolver, entry *symbols.Symbol) (warnings []*Error, err error) { + _, warnings, err = compile(index, model, resolver, entry, true) + return warnings, err +} + +func compile(index *symbols.Index, model *semantics.Model, resolver *resolve.Resolver, entry *symbols.Symbol, lenient bool) (*Plan, []*Error, error) { if index == nil || model == nil || resolver == nil { - return nil, &Error{Kind: ErrorInvalidContext} + return nil, nil, &Error{Kind: ErrorInvalidContext} } all := bases{ document: libraryBase(index, documentBaseFQN), @@ -133,11 +180,11 @@ func Compile(index *symbols.Index, model *semantics.Model, resolver *resolve.Res all.table == nil || all.list == nil || all.definitions == nil || all.formula == nil || all.diagram == nil || all.image == nil || all.run == nil || all.span == nil || all.link == nil || all.ref == nil || all.columnRun == nil || all.spanColumn == nil || all.linkColumn == nil { - return nil, &Error{Kind: ErrorLibraryUnavailable} + return nil, nil, &Error{Kind: ErrorLibraryUnavailable} } name := symbols.FQNOf(entry) if entry == nil || entry == all.document || entry.Kind != symbols.SymbolPartDef || !model.Conforms(entry, all.document) { - return nil, &Error{Kind: ErrorNotDocumentDefinition, Document: name, Origin: entry.Origin()} + return nil, nil, &Error{Kind: ErrorNotDocumentDefinition, Document: name, Origin: entry.Origin()} } c := &compiler{ index: index, @@ -149,17 +196,21 @@ func Compile(index *symbols.Index, model *semantics.Model, resolver *resolve.Res targets: make(map[*symbols.Symbol]refTarget), parsed: make(map[*symbols.Symbol]*parsedBody), outlines: make(map[*symbols.Symbol]*markupOutline), + + lenient: lenient, + scanned: entry, + uncheckedBy: make(map[uncheckedKey]*Error), } title, err := c.requiredText(entry, "title", ErrorMissingTitle) if err != nil { - return nil, err + return nil, c.unchecked, err } content, err := c.compileMembers(entry) if err != nil { - return nil, err + return nil, c.unchecked, err } if err := c.resolveRefs(content); err != nil { - return nil, err + return nil, c.unchecked, err } return &Plan{ compiled: true, @@ -167,7 +218,7 @@ func Compile(index *symbols.Index, model *semantics.Model, resolver *resolve.Res title: title, content: content, origin: entry.Origin(), - }, nil + }, c.unchecked, nil } func libraryBase(index *symbols.Index, fqn string) *symbols.Symbol { @@ -372,13 +423,14 @@ func (c *compiler) compileParagraph(member *symbols.Symbol) (Content, error) { Origin: member.Origin(), } } + var unchecked bool if len(bodies) > 0 { - runs, err = c.compileParagraphBody(member, bodies) + runs, unchecked, err = c.compileParagraphBody(member, bodies) if err != nil { return Content{}, err } } - if !stated && query == nil && len(runs) == 0 { + if !stated && query == nil && len(runs) == 0 && !unchecked { return Content{}, &Error{ Kind: ErrorMissingText, Document: c.document, diff --git a/internal/ir/docplan/errors.go b/internal/ir/docplan/errors.go index 8d2231e0de..bd3974e984 100644 --- a/internal/ir/docplan/errors.go +++ b/internal/ir/docplan/errors.go @@ -5,6 +5,7 @@ import ( "github.com/Open-MBEE/OpenSysML/internal/ir/view" "github.com/Open-MBEE/OpenSysML/internal/semantic/symbols" + "github.com/Open-MBEE/OpenSysML/internal/syntax/diag" ) // ErrorKind classifies a document-planning failure. @@ -90,7 +91,19 @@ type Error struct { Origin symbols.Origin // Line is the 1-based line within a markup body the failure is at, or 0. Line int - Err error + // Bodies counts the bodies a missing markup converter left unchecked when + // validation went on without it, or is 0 when planning stopped at the body. + Bodies int + Err error +} + +// Severity is how a check reports an error of this kind: a missing markup +// converter leaves bodies unchecked, a warning; every other kind is an error. +func (k ErrorKind) Severity() diag.Severity { + if k == ErrorMarkupToolMissing { + return diag.SeverityWarning + } + return diag.SeverityError } func (e *Error) Error() string { @@ -270,6 +283,12 @@ func (e *Error) message() string { case ErrorMarkupBodyNotParagraph: return fmt.Sprintf("document %s paragraph %s carries a %s body that yields %s rather than exactly one paragraph", e.Document, e.Content, e.Parameter, e.Actual) case ErrorMarkupToolMissing, ErrorMarkupToolFailed: + switch { + case e.Bodies > 1: + return fmt.Sprintf("document %s carries %d %s bodies that were not checked, the first in content %s: %v", e.Document, e.Bodies, e.Parameter, e.Content, e.Err) + case e.Bodies == 1: + return fmt.Sprintf("document %s content %s carries a %s body that was not checked: %v", e.Document, e.Content, e.Parameter, e.Err) + } return fmt.Sprintf("document %s content %s carries a %s body that could not be parsed: %v", e.Document, e.Content, e.Parameter, e.Err) case ErrorMarkupHeadingLevel: return fmt.Sprintf("document %s content %s carries a %s body whose %s is out of sequence: expected %s", e.Document, e.Content, e.Parameter, e.Actual, e.Expected) diff --git a/internal/ir/docplan/markup/asciidoc.go b/internal/ir/docplan/markup/asciidoc.go index a048a2cf93..9c9abf75ec 100644 --- a/internal/ir/docplan/markup/asciidoc.go +++ b/internal/ir/docplan/markup/asciidoc.go @@ -3,6 +3,7 @@ package markup import ( "encoding/json" "fmt" + "os" "strings" "time" ) @@ -15,6 +16,15 @@ const AsciiDocEnv = "OPENSYSML_ASCIIDOCTOR" // asciiDocTool is the converter's name on PATH when the variable is unset. const asciiDocTool = "asciidoctor-json" +// asciiDocName is the converter to look for: what the variable names, and +// whether it named one, else the default to look up on PATH. +func asciiDocName() (name string, named bool) { + if name = strings.TrimSpace(os.Getenv(AsciiDocEnv)); name != "" { + return name, true + } + return asciiDocTool, false +} + // asciiDocTimeout bounds one converter run, as every document converter is bounded. const asciiDocTimeout = 5 * time.Minute @@ -28,6 +38,9 @@ type AsciiDoc struct{} // Language implements Parser. func (AsciiDoc) Language() string { return "asciidoc" } +// External reports that the converter runs outside the process. +func (AsciiDoc) External() bool { return true } + // Parse implements Parser. func (AsciiDoc) Parse(body string) ([]Block, error) { output, err := runAsciiDoc(body) @@ -37,10 +50,13 @@ func (AsciiDoc) Parse(body string) ([]Block, error) { return decodeAsciiDoc(body, output) } -// ToolError reports the converter missing or failing. +// ToolError reports the converter missing or failing: Tool is the converter +// looked for, Named whether EnvVar named it (else it is the default looked up +// on PATH), Missing whether it was not found, Detail what else was observed. type ToolError struct { Tool string EnvVar string + Named bool Missing bool Detail string } @@ -48,8 +64,13 @@ type ToolError struct { func (e *ToolError) Error() string { if e.Missing { msg := fmt.Sprintf("the AsciiDoc converter %s was not found", e.Tool) - if e.Detail != "" { + switch { + case e.Detail != "": msg += " (" + e.Detail + ")" + case e.Named: + msg += fmt.Sprintf(" (%s names it)", e.EnvVar) + default: + msg += fmt.Sprintf(" on PATH (%s is unset)", e.EnvVar) } return msg + fmt.Sprintf("; run ./scripts/download-doc-pdf-toolchain.sh and point %s at build/doc-pdf/asciidoctor/asciidoctor-json.js", e.EnvVar) } diff --git a/internal/ir/docplan/markup/asciidoc_exec.go b/internal/ir/docplan/markup/asciidoc_exec.go index 76f30af1b2..9f80546a1b 100644 --- a/internal/ir/docplan/markup/asciidoc_exec.go +++ b/internal/ir/docplan/markup/asciidoc_exec.go @@ -7,7 +7,6 @@ import ( "context" "errors" "fmt" - "os" "os/exec" "path/filepath" "strings" @@ -16,17 +15,14 @@ import ( // locateAsciiDoc finds the converter: the executable the variable names, else // the tool on PATH. func locateAsciiDoc() (string, error) { - name := strings.TrimSpace(os.Getenv(AsciiDocEnv)) - if name == "" { - name = asciiDocTool - } + name, named := asciiDocName() path, err := exec.LookPath(name) if err != nil { - return "", &ToolError{Tool: name, EnvVar: AsciiDocEnv, Missing: true} + return "", &ToolError{Tool: name, EnvVar: AsciiDocEnv, Named: named, Missing: true} } if !filepath.IsAbs(path) { if path, err = filepath.Abs(path); err != nil { - return "", &ToolError{Tool: name, EnvVar: AsciiDocEnv, Missing: true, Detail: err.Error()} + return "", &ToolError{Tool: name, EnvVar: AsciiDocEnv, Named: named, Missing: true, Detail: err.Error()} } } return path, nil diff --git a/internal/ir/docplan/markup/asciidoc_test.go b/internal/ir/docplan/markup/asciidoc_test.go index a2833e12ca..cc384ee9bd 100644 --- a/internal/ir/docplan/markup/asciidoc_test.go +++ b/internal/ir/docplan/markup/asciidoc_test.go @@ -302,11 +302,51 @@ func TestAsciiDocMissingConverter(t *testing.T) { t.Fatalf("error = %v, want the converter missing", err) } msg := tool.Error() - for _, want := range []string{"download-doc-pdf-toolchain.sh", AsciiDocEnv, "no-such-converter"} { + for _, want := range []string{"download-doc-pdf-toolchain.sh", "no-such-converter", "(" + AsciiDocEnv + " names it)"} { if !strings.Contains(msg, want) { t.Errorf("message %q lacks %q", msg, want) } } + if strings.Contains(msg, "PATH") { + t.Errorf("message %q blames PATH for a converter the variable named", msg) + } +} + +func TestAsciiDocMissingDefaultConverterNamesPath(t *testing.T) { + t.Setenv(AsciiDocEnv, "") + t.Setenv("PATH", t.TempDir()) + _, err := AsciiDoc{}.Parse("a\n") + var tool *ToolError + if !errors.As(err, &tool) || !tool.Missing || tool.Named || tool.Tool != asciiDocTool { + t.Fatalf("error = %v, want the default converter %s missing", err, asciiDocTool) + } + msg := tool.Error() + for _, want := range []string{"the AsciiDoc converter asciidoctor-json was not found on PATH (" + AsciiDocEnv + " is unset)", "; run ./scripts/download-doc-pdf-toolchain.sh and point " + AsciiDocEnv + " at build/doc-pdf/asciidoctor/asciidoctor-json.js"} { + if !strings.Contains(msg, want) { + t.Errorf("message %q lacks %q", msg, want) + } + } +} + +func TestToolErrorMessages(t *testing.T) { + cases := map[string]struct { + err ToolError + want string + }{ + "missing with detail": { + ToolError{Tool: "x", EnvVar: AsciiDocEnv, Named: true, Missing: true, Detail: "this host starts no process"}, + "the AsciiDoc converter x was not found (this host starts no process); run ./scripts/download-doc-pdf-toolchain.sh and point " + AsciiDocEnv + " at build/doc-pdf/asciidoctor/asciidoctor-json.js", + }, + "failed": { + ToolError{Tool: "x", EnvVar: AsciiDocEnv, Detail: "exit status 1"}, + "the AsciiDoc converter x failed: exit status 1", + }, + } + for name, c := range cases { + if got := c.err.Error(); got != c.want { + t.Errorf("%s: message %q, want %q", name, got, c.want) + } + } } // fakeConverter writes an executable that answers stdout and stderr and exits diff --git a/internal/ir/docplan/markup/asciidoc_wasm.go b/internal/ir/docplan/markup/asciidoc_wasm.go index 3895c0f5e8..eabe8aee5b 100644 --- a/internal/ir/docplan/markup/asciidoc_wasm.go +++ b/internal/ir/docplan/markup/asciidoc_wasm.go @@ -2,17 +2,13 @@ package markup -import ( - "os" - "strings" -) - // runAsciiDoc reports the converter missing: no WebAssembly host starts a // process, and leaving os/exec out keeps the core build within its size budget. func runAsciiDoc(string) ([]byte, error) { - name := strings.TrimSpace(os.Getenv(AsciiDocEnv)) - if name == "" { - name = asciiDocTool + name, named := asciiDocName() + detail := "this host starts no process, so PATH was not searched" + if named { + detail = AsciiDocEnv + " names it, but this host starts no process" } - return nil, &ToolError{Tool: name, EnvVar: AsciiDocEnv, Missing: true, Detail: "this host starts no process"} + return nil, &ToolError{Tool: name, EnvVar: AsciiDocEnv, Named: named, Missing: true, Detail: detail} } diff --git a/internal/ir/docplan/markup/markdown.go b/internal/ir/docplan/markup/markdown.go index e6601fe8a3..e1b83c6618 100644 --- a/internal/ir/docplan/markup/markdown.go +++ b/internal/ir/docplan/markup/markdown.go @@ -21,6 +21,9 @@ type Markdown struct{} // Language implements Parser. func (Markdown) Language() string { return "markdown" } +// External reports that Markdown is parsed in this process. +func (Markdown) External() bool { return false } + // markdown is the configured CommonMark parser: the default block and inline // parsers plus inline mathematics. var markdown = goldmark.New(goldmark.WithParserOptions( diff --git a/internal/ir/docplan/markup/markup.go b/internal/ir/docplan/markup/markup.go index c27b50fb29..16937a040a 100644 --- a/internal/ir/docplan/markup/markup.go +++ b/internal/ir/docplan/markup/markup.go @@ -16,6 +16,9 @@ type Parser interface { // Parse maps a body onto content blocks, or fails with an // *UnsupportedConstructError for a construct no content block carries. Parse(body string) ([]Block, error) + // External reports whether Parse runs a converter of the host rather than + // code of this process, so that what it answers is the host's. + External() bool } // For returns the parser for a textual representation's language, matched diff --git a/internal/ir/docplan/markup_bodies.go b/internal/ir/docplan/markup_bodies.go index d33a6ace35..01aacd6039 100644 --- a/internal/ir/docplan/markup_bodies.go +++ b/internal/ir/docplan/markup_bodies.go @@ -34,9 +34,10 @@ func markupLanguage(rep *symbols.Symbol) string { // compileParagraphBody compiles the one markup body a paragraph carries into // its runs: the body must yield exactly one paragraph, whose runs they are. -func (c *compiler) compileParagraphBody(member *symbols.Symbol, bodies []*symbols.Symbol) ([]Run, error) { +// A body left unchecked for want of its converter yields no runs and says so. +func (c *compiler) compileParagraphBody(member *symbols.Symbol, bodies []*symbols.Symbol) (runs []Run, unchecked bool, err error) { if len(bodies) > 1 { - return nil, &Error{ + return nil, false, &Error{ Kind: ErrorMarkupBodyNotParagraph, Document: c.document, Content: symbols.FQNOf(member), @@ -45,12 +46,15 @@ func (c *compiler) compileParagraphBody(member *symbols.Symbol, bodies []*symbol Origin: bodies[1].Origin(), } } + if c.parseMarkupBody(member, bodies[0]).unchecked != nil { + return nil, true, nil + } blocks, err := c.compileMarkupBlocks(member, bodies[0]) if err != nil { - return nil, err + return nil, false, err } if len(blocks) != 1 || blocks[0].kind != ContentParagraph { - return nil, &Error{ + return nil, false, &Error{ Kind: ErrorMarkupBodyNotParagraph, Document: c.document, Content: symbols.FQNOf(member), @@ -59,7 +63,7 @@ func (c *compiler) compileParagraphBody(member *symbols.Symbol, bodies []*symbol Origin: bodies[0].Origin(), } } - return blocks[0].runs, nil + return blocks[0].runs, false, nil } // describeBlocks states what a body yielded, for a paragraph that wanted one paragraph. diff --git a/internal/ir/docplan/markup_sections.go b/internal/ir/docplan/markup_sections.go index 70a03384d9..7bcc4a4614 100644 --- a/internal/ir/docplan/markup_sections.go +++ b/internal/ir/docplan/markup_sections.go @@ -60,13 +60,15 @@ func (o *markupOutline) sectionAt(path []string) (markupSection, bool) { } // parsedBody is a textual representation's body parsed once: its language, -// text, offsets into the source and blocks, or the planning error instead. +// text, offsets into the source and blocks, or the planning error instead; +// unchecked is the warning a lenient plan left the body unparsed under. type parsedBody struct { - language markup.Parser - body string - offsets []int - blocks []markup.Block - err *Error + language markup.Parser + body string + offsets []int + blocks []markup.Block + err *Error + unchecked *Error } // at is the source origin of a span of the body. @@ -118,6 +120,12 @@ func (c *compiler) parseBody(owner, rep *symbols.Symbol) *parsedBody { } body, offsets := source.CommentBodyOffsets(lookup(rep.DocName, decl.BodySpan)) parsed := &parsedBody{language: language, body: body, offsets: offsets} + key := uncheckedKey{c.scanned, language.Language()} + if prior := c.uncheckedBy[key]; prior != nil { + prior.Bodies++ + parsed.unchecked = prior + return parsed + } blocks, err := language.Parse(body) if err == nil { parsed.blocks = blocks @@ -129,7 +137,18 @@ func (c *compiler) parseBody(owner, rep *symbols.Symbol) *parsedBody { if tool.Missing { kind = ErrorMarkupToolMissing } - parsed.err = &Error{Kind: kind, Document: c.document, Content: symbols.FQNOf(owner), Parameter: language.Language(), Origin: rep.Origin(), Err: tool} + planning := &Error{Kind: kind, Document: symbols.FQNOf(c.scanned), Content: symbols.FQNOf(owner), Parameter: language.Language(), Origin: rep.Origin(), Err: tool} + if c.lenient && tool.Missing { + // Another document's bodies are its own plan's to warn of. + planning.Bodies = 1 + if c.scanned == c.entry { + c.unchecked = append(c.unchecked, planning) + } + c.uncheckedBy[key] = planning + parsed.unchecked = planning + return parsed + } + parsed.err = planning return parsed } var unsupported *markup.UnsupportedConstructError @@ -157,7 +176,10 @@ func (c *compiler) outlineOf(doc *symbols.Symbol) *markupOutline { c.outlines[doc] = outline used := make(map[string]bool) c.collectContentNames(doc, used) + scanned := c.scanned + c.scanned = doc c.scanBodies(doc, nil, outline, used) + c.scanned = scanned return outline } diff --git a/internal/workspace/model/examples_test.go b/internal/workspace/model/examples_test.go index 9ff9543880..cc60926527 100644 --- a/internal/workspace/model/examples_test.go +++ b/internal/workspace/model/examples_test.go @@ -13,8 +13,8 @@ import ( const examplesDir = "../../../examples" -// markupToolMissingCode is the document-plan error for a prose body whose -// converter is not installed; toolchainRequiredEnv turns the skip into a failure. +// markupToolMissingCode is the document-plan warning for a prose body whose +// converter is not installed; toolchainRequiredEnv makes that a failure. const ( markupToolMissingCode = "document-plan-" + string(docplan.ErrorMarkupToolMissing) toolchainRequiredEnv = "OPENSYSML_REQUIRE_PDF_TOOLCHAIN" @@ -41,25 +41,25 @@ func TestExamplesAnalyseCleanly(t *testing.T) { ws.Open(sibling, content, 1) } - var errs []string - toolMissing := true + var errs, unchecked []string for _, d := range ws.Diagnostics(rel) { - if d.Severity == diag.SeverityError { + switch { + case d.Severity == diag.SeverityError: errs = append(errs, d.Message) - toolMissing = toolMissing && d.Code == markupToolMissingCode + case d.Code == markupToolMissingCode: + unchecked = append(unchecked, d.Message) } } - - if len(errs) > 0 && toolMissing { + if len(errs) > 0 { + t.Errorf("%d error(s): %s", len(errs), strings.Join(errs, "; ")) + } + if len(unchecked) > 0 { // The example's prose needs an external converter: the PDF - // toolchain job installs it and requires the example to analyse. + // toolchain job installs it and requires the bodies checked. if required := os.Getenv(toolchainRequiredEnv); required != "" { - t.Fatalf("%s=%s but %s", toolchainRequiredEnv, required, strings.Join(errs, "; ")) + t.Fatalf("%s=%s but %s", toolchainRequiredEnv, required, strings.Join(unchecked, "; ")) } - t.Skipf("%s", strings.Join(errs, "; ")) - } - if len(errs) > 0 { - t.Errorf("%d error(s): %s", len(errs), strings.Join(errs, "; ")) + t.Logf("left unchecked: %s", strings.Join(unchecked, "; ")) } }) } diff --git a/internal/workspace/model/record.go b/internal/workspace/model/record.go index 9c8355d2e7..8b49875931 100644 --- a/internal/workspace/model/record.go +++ b/internal/workspace/model/record.go @@ -7,9 +7,15 @@ import ( "slices" "github.com/Open-MBEE/OpenSysML/internal/check/passes" + "github.com/Open-MBEE/OpenSysML/internal/check/passes/kit" + "github.com/Open-MBEE/OpenSysML/internal/ir/docplan" + "github.com/Open-MBEE/OpenSysML/internal/ir/docplan/markup" + "github.com/Open-MBEE/OpenSysML/internal/ir/queryplan" "github.com/Open-MBEE/OpenSysML/internal/semantic/resolve" "github.com/Open-MBEE/OpenSysML/internal/semantic/symbols" + "github.com/Open-MBEE/OpenSysML/internal/syntax/ast" "github.com/Open-MBEE/OpenSysML/internal/syntax/diag" + "github.com/Open-MBEE/OpenSysML/internal/syntax/source" "github.com/Open-MBEE/OpenSysML/internal/workspace/libs" ) @@ -50,6 +56,29 @@ func (w *Workspace) InterfaceRecord(name string) (*libs.InterfaceRecord, error) return w.interfaceRecordLocked(name, doc, w.sourcesLocked()) } +// plannedElsewhereLocked refuses the record of a document whose query definitions or +// externally-converted prose bodies another document's plan reads. Caller holds the write lock. +func (w *Workspace) plannedElsewhereLocked(name string) error { + root := w.index.DocumentRoot(name) + if root == nil { + return nil + } + _, sem := w.semanticsLocked() + for _, sym := range kit.Symbols(nil, root) { + if queryplan.IsQueryDefinition(w.index, sem, sym) { + return fmt.Errorf("%w: result expression of document query definition %s", libs.ErrUnrecordable, symbols.FQNOf(sym)) + } + decl, ok := sym.Decl.(*ast.TextualRepresentation) + if !ok || !docplan.ReadsBody(w.index, sem, sym) { + continue + } + if parser, ok := markup.For(source.StringValue(decl.Language)); ok && parser.External() { + return fmt.Errorf("%w: %s body of %s, which an external converter checks", libs.ErrUnrecordable, parser.Language(), symbols.FQNOf(sym)) + } + } + return nil +} + // interfaceRecordLocked is InterfaceRecord for a loaded document, attributing // its provenance against src, which a caller writing many records shares. // Caller holds the write lock. @@ -62,6 +91,9 @@ func (w *Workspace) interfaceRecordLocked(name string, doc *Document, src libs.S delete(w.batched, name) delete(w.diagCache, name) } + if err := w.plannedElsewhereLocked(name); err != nil { + return nil, err + } diags := w.diagnosticsLocked(name, doc) gathered, err := passes.GatherRelationships(w.contextLocked(), name) if err != nil { diff --git a/internal/workspace/model/record_host_dependent_test.go b/internal/workspace/model/record_host_dependent_test.go new file mode 100644 index 0000000000..614dd8e952 --- /dev/null +++ b/internal/workspace/model/record_host_dependent_test.go @@ -0,0 +1,97 @@ +package model + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/Open-MBEE/OpenSysML/internal/ir/docplan/markup" + "github.com/Open-MBEE/OpenSysML/internal/ir/docplan/markup/markuptest" + "github.com/Open-MBEE/OpenSysML/internal/workspace/libs" +) + +const proseWithAsciiDocBody = `package Prose { + private import DocumentQueries::*; + part def Report :> Document { + attribute redefines title = "Report"; + part note : Paragraph { + rep language "asciidoc" /* Mind the *gap*. */ + } + } +}` + +// A document with a body an external converter checks is not recorded, +// whether the converter was missing — the warning is this host's — or found +// and the body checked: the verdict would not hold on a host without it. +func TestInterfaceRecordRefusesExternallyCheckedBodies(t *testing.T) { + t.Run("converter missing", func(t *testing.T) { + t.Setenv(markup.AsciiDocEnv, filepath.Join(t.TempDir(), "missing-converter")) + ws := NewWorkspace() + ws.Open("prose.sysml", []byte(proseWithAsciiDocBody), 1) + var warned bool + for _, d := range ws.Diagnostics("prose.sysml") { + warned = warned || d.Code == markupToolMissingCode + } + if !warned { + t.Fatal("the document reports no missing converter: the fixture is vacuous") + } + if _, err := ws.InterfaceRecord("prose.sysml"); !errors.Is(err, libs.ErrUnrecordable) { + t.Fatalf("InterfaceRecord = %v, want %v", err, libs.ErrUnrecordable) + } + }) + t.Run("converter found", func(t *testing.T) { + converter, err := markuptest.RecordedConverter(t.TempDir(), "../../ir/docplan/markup/testdata") + if err != nil { + t.Skip(err) + } + t.Setenv(markup.AsciiDocEnv, converter) + // The recorded converter answers the bodies of this fixture. + content, err := os.ReadFile("../../doc/docrender/testdata/asciidoc_report.sysml") + if err != nil { + t.Fatal(err) + } + ws := NewWorkspace() + ws.Open("prose.sysml", content, 1) + if diags := ws.Diagnostics("prose.sysml"); len(diags) != 0 { + t.Fatalf("the checked document reports %v, want nothing", diags) + } + if _, err := ws.InterfaceRecord("prose.sysml"); !errors.Is(err, libs.ErrUnrecordable) { + t.Fatalf("InterfaceRecord = %v, want %v", err, libs.ErrUnrecordable) + } + }) +} + +// A body no document plan reads — one on an ordinary part — leaves the +// document recordable, converter or no converter. +func TestInterfaceRecordKeepsBodiesOutsideDocuments(t *testing.T) { + t.Setenv(markup.AsciiDocEnv, filepath.Join(t.TempDir(), "missing-converter")) + ws := NewWorkspace() + ws.Open("engine.sysml", []byte(`package Model { part engine { rep language "asciidoc" /* engine notes */ } }`), 1) + if diags := ws.Diagnostics("engine.sysml"); len(diags) != 0 { + t.Fatalf("the fixture reports %v, want nothing", diags) + } + if _, err := ws.InterfaceRecord("engine.sysml"); err != nil { + t.Fatalf("InterfaceRecord = %v, want the record", err) + } +} + +// A document declaring a document query definition is not recorded: a plan +// in another document evaluates the query's result expression from the tree. +func TestInterfaceRecordRefusesQueryDefinitions(t *testing.T) { + ws := NewWorkspace() + ws.Open("queries.sysml", []byte(`package Queries { + private import DocumentQueries::*; + private import KerML::Root::Element; + calc def Names :> Query { + in root : Element; + OwnedElements(source = root) + } +}`), 1) + if diags := ws.Diagnostics("queries.sysml"); len(diags) != 0 { + t.Fatalf("the fixture reports %v, want nothing", diags) + } + if _, err := ws.InterfaceRecord("queries.sysml"); !errors.Is(err, libs.ErrUnrecordable) { + t.Fatalf("InterfaceRecord = %v, want %v", err, libs.ErrUnrecordable) + } +}