Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions api/proto/sysml.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions api/proto/sysml.proto
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,9 @@ message Diagnostic {
string severity = 1; // "error", "warning", "info"
string message = 2;
Span span = 3;
// Stable identifier to branch on instead of the message: a pass or rule code,
// "syntax", "choice-point", "guard-unevaluable"; empty when none was assigned.
string code = 4;
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
}

// Span represents a source location
Expand Down Expand Up @@ -911,6 +914,8 @@ message ServerInfoResponse {
// and answers with typed rows.
// "render_document" - the RenderDocument RPC renders a named document to
// Markdown.
// "diagnostic_codes" - Diagnostic.code is populated, so an empty code is a
// finding none was assigned; without it every code is empty.
repeated string capabilities = 2;
}

Expand Down
7 changes: 7 additions & 0 deletions changes/unreleased/diagnostic-wire-code.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
- **A wire `Diagnostic` carries its `code`.** The gRPC/Connect `Diagnostic` message gains
`string code = 4`, the stable identifier the runtime already assigns: `syntax` for a parse
error, the pass or rule code for a validation finding, `choice-point` and `guard-unevaluable`
for a run's notes. Every response that carries diagnostics carries it, so a client branches on
`code` instead of a message prefix; a diagnostic whose producer assigned no code sends it empty.
A service that populates it advertises the `diagnostic_codes` capability. The Go, Python,
Node, Rust and Java clients expose it as `Diagnostic.code` and name the capability.
30 changes: 30 additions & 0 deletions client/opensysml/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,36 @@ func TestASyntaxErrorIsADiagnosticNotAnError(t *testing.T) {
if len(model.Diagnostics) == 0 {
t.Error("broken source parsed without diagnostics")
}
for _, diag := range model.Diagnostics {
if diag.Code != "syntax" {
t.Errorf("syntax error coded %q, want syntax: %s", diag.Code, diag.Message)
}
}
}

func TestADiagnosticCarriesItsCode(t *testing.T) {
client := newClient(t)
info, err := client.ServerInfo(context.Background())
if err != nil {
t.Fatalf("ServerInfo: %v", err)
}
if !info.Has(opensysml.CapabilityDiagnosticCodes) {
t.Errorf("capabilities %v do not include %s", info.Capabilities, opensysml.CapabilityDiagnosticCodes)
}
model, err := client.ParseSource(context.Background(), "package P { part def W { part hub : Missing; } }")
if err != nil {
t.Fatalf("ParseSource: %v", err)
}
found := false
for _, diag := range model.Diagnostics {
if diag.Code == "" {
t.Errorf("diagnostic without a code: %s", diag.Message)
}
found = found || diag.Code == "unresolved"
}
if !found {
t.Errorf("no unresolved diagnostic among %v", model.Diagnostics)
}
}

func TestAMissingFileIsNotFound(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion client/opensysml/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func diagnosticsFromProto(diags []*pb.Diagnostic) []Diagnostic {
}
out := make([]Diagnostic, 0, len(diags))
for _, diag := range diags {
converted := Diagnostic{Severity: diag.Severity, Message: diag.Message}
converted := Diagnostic{Severity: diag.Severity, Message: diag.Message, Code: diag.Code}
if diag.Span != nil {
converted.Span = &Span{
File: diag.Span.File,
Expand Down
4 changes: 4 additions & 0 deletions client/opensysml/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const (
CapabilityStructuredValues = sysmlgrpc.CapabilityStructuredValues
CapabilityMeasurementRefs = sysmlgrpc.CapabilityMeasurementRefs
CapabilityFunctionValues = sysmlgrpc.CapabilityFunctionValues
CapabilityDiagnosticCodes = sysmlgrpc.CapabilityDiagnosticCodes
CapabilityVerificationVerdicts = sysmlgrpc.CapabilityVerificationVerdicts
)

Expand Down Expand Up @@ -175,6 +176,9 @@ type Diagnostic struct {
// Severity is SeverityError, SeverityWarning or SeverityInfo.
Severity string
Message string
// Code identifies what was found, stable across message wording ("syntax", a
// validation code, "choice-point", "guard-unevaluable"); empty when none was assigned.
Code string
// Span locates the finding in its source, nil when it has no location.
Span *Span
}
Expand Down
7 changes: 6 additions & 1 deletion clients/java/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ records (`IntegerValue`, `RealValue`, `ComplexValue`, `QuantityValue`, `ArrayVal
`VectorValue`, `VectorQuantityValue`, `MeasurementRefValue`, `FunctionValue`, `EnumerationValue`,
`InstanceReference`, `Sequence`, `NullValue`, `UnsetValue`, …), and `Symbol`,
`Diagnostic`, `Instance` and `Instantiation` are records with copied collections.
No generated protobuf message or builder appears in the public API.
No generated protobuf message or builder appears in the public API. A `Diagnostic`
is `(severity, message, code, span)`; `code()` is the identifier to branch on
(`"syntax"`, a validation code such as `"unresolved"`, `"choice-point"`,
`"guard-unevaluable"`; `""` when the service assigned none), `message()` is for reading. A
service that populates `code` advertises `Capabilities.DIAGNOSTIC_CODES`; without it every
code is `""`.

## Exceptions: unchecked, and the distinction that matters

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ public final class Capabilities {
/** The unbounded value {@code *} travels as itself rather than as an unsupported null. */
public static final String INFINITY_VALUE = "infinity_value";

/** {@code Diagnostic.code} is populated, so an empty code is a finding none was assigned. */
public static final String DIAGNOSTIC_CODES = "diagnostic_codes";

/** The {@code ApplyEdits} RPC edits a parsed model's own source. */
public static final String APPLY_EDITS = "apply_edits";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,25 @@
*
* @param severity how serious the finding is
* @param message the finding, as the service words it
* @param code stable identifier of what was found, independent of the message wording: a
* validation code, {@code "syntax"}, or {@code "choice-point"} / {@code "guard-unevaluable"}
* on a run; empty when the service assigned none
* @param span where in the source it is, absent when the service located none
*/
public record Diagnostic(Severity severity, String message, Optional<Span> span) {
public record Diagnostic(Severity severity, String message, String code, Optional<Span> span) {

/**
* Creates a diagnostic.
*
* @param severity the severity, never {@code null}
* @param message the message, never {@code null}
* @param code the code, empty rather than {@code null} when there is none
* @param span the source location, absent when unlocated
*/
public Diagnostic {
Objects.requireNonNull(severity, "severity");
Objects.requireNonNull(message, "message");
Objects.requireNonNull(code, "code");
Objects.requireNonNull(span, "span");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
*/
public class ModelException extends OpenSysMLException {

private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;
private static final int MAX_SERIALIZED_DIAGNOSTICS = 100_000;

private transient List<Diagnostic> diagnostics;
Expand Down Expand Up @@ -55,6 +55,7 @@ private void writeObject(ObjectOutputStream stream) throws IOException {
for (Diagnostic diagnostic : diagnostics) {
stream.writeObject(diagnostic.severity());
stream.writeObject(diagnostic.message());
stream.writeObject(diagnostic.code());
stream.writeBoolean(diagnostic.span().isPresent());
if (diagnostic.span().isPresent()) {
Diagnostic.Span span = diagnostic.span().orElseThrow();
Expand All @@ -81,8 +82,10 @@ private void readObject(ObjectInputStream stream) throws IOException, ClassNotFo
for (int index = 0; index < count; index++) {
Object severity = stream.readObject();
Object message = stream.readObject();
Object code = stream.readObject();
if (!(severity instanceof Diagnostic.Severity diagnosticSeverity)
|| !(message instanceof String diagnosticMessage)) {
|| !(message instanceof String diagnosticMessage)
|| !(code instanceof String diagnosticCode)) {
throw new InvalidObjectException("invalid diagnostic");
}
Optional<Diagnostic.Span> span = Optional.empty();
Expand All @@ -100,7 +103,7 @@ private void readObject(ObjectInputStream stream) throws IOException, ClassNotFo
stream.readInt(),
stream.readInt()));
}
restored.add(new Diagnostic(diagnosticSeverity, diagnosticMessage, span));
restored.add(new Diagnostic(diagnosticSeverity, diagnosticMessage, diagnosticCode, span));
}
diagnostics = List.copyOf(restored);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ public static List<Diagnostic> diagnostics(
new Diagnostic(
Diagnostic.Severity.fromWireName(diagnostic.getSeverity()),
diagnostic.getMessage(),
diagnostic.getCode(),
diagnostic.hasSpan() ? Optional.of(span(diagnostic.getSpan())) : Optional.empty()));
}
return List.copyOf(read);
Expand Down
Loading
Loading