From a4b2475d23e01e2ae9f8fdcfedcc6caf496a8aa1 Mon Sep 17 00:00:00 2001 From: Jan-Hendrik Spahn Date: Wed, 8 Jul 2026 17:05:33 +0200 Subject: [PATCH 1/2] feat(cimvocabcheck): warn on unused SPARQL variables (GH-23) - new schema-independent UnusedVariableCheck over the parsed syntax tree, reporting two WARN codes: PROJECTED_VARIABLE_UNBOUND (SELECT list / CONSTRUCT template / DESCRIBE variable that appears nowhere in the query body) and UNUSED_VARIABLE (variable bound in a triple pattern, BIND or VALUES that occurs exactly once in its scope) - quiet on idiomatic SPARQL: SELECT */DESCRIBE * and ASK bodies, single-use variables inside EXISTS/NOT EXISTS/MINUS, variable predicates, GRAPH/ SERVICE names, sub-SELECT projections; per-SELECT-scope analysis; unknown syntax forms skip the check instead of risking false positives - runs in schema-aware validation and in the checkSyntaxOnly fallback; SPARQL Update requests are not checked - SHACL embedded SPARQL: pre-bound variables ($this, ?value, $PATH, $shapesGraph, $currentShape) are exempted in both the schema-aware and syntax-only paths - SourceLocator.locateVariable finds the ?name/$name token for positions - docs: new "Unused-variable checks" section in validation-checks.md Co-Authored-By: Claude Fable 5 --- .../cimvocabcheck/core/SourceLocator.java | 15 + .../core/SparqlQueryValidator.java | 1 + .../core/SparqlValidationApi.java | 54 +- .../core/SparqlValidationCode.java | 14 +- .../core/UnusedVariableCheck.java | 461 ++++++++++++++++++ .../core/UnusedVariableCheckTest.java | 331 +++++++++++++ .../cimvocabcheck/validation-checks.md | 31 ++ 7 files changed, 899 insertions(+), 8 deletions(-) create mode 100644 cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/UnusedVariableCheck.java create mode 100644 cimvocabcheck/core/src/test/java/de/soptim/opencgmes/cimvocabcheck/core/UnusedVariableCheckTest.java diff --git a/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SourceLocator.java b/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SourceLocator.java index ab4b6c50..44b4890d 100644 --- a/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SourceLocator.java +++ b/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SourceLocator.java @@ -168,6 +168,21 @@ public static Location locateWithHint( return toLineColumn(query, best); } + /** + * Locates the first occurrence of a SPARQL variable in the query text, trying both the {@code + * ?name} and {@code $name} spellings. Returns {@link #UNKNOWN} when the variable cannot be found. + */ + public static Location locateVariable(String query, String name) { + if (query == null || name == null || name.isEmpty()) { + return UNKNOWN; + } + var offsets = new ArrayList(); + collectAllWholeToken(query, "?" + name, offsets); + collectAllWholeToken(query, "$" + name, offsets); + offsets.sort(Integer::compare); + return offsets.isEmpty() ? UNKNOWN : toLineColumn(query, offsets.get(0)); + } + // ---- offset collection ----------------------------------------------------------------- private static List findAllTermOffsets(String query, Node term, PrefixMapping prefixes) { diff --git a/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlQueryValidator.java b/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlQueryValidator.java index ff538f6f..63d1949f 100644 --- a/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlQueryValidator.java +++ b/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlQueryValidator.java @@ -152,6 +152,7 @@ public SparqlValidationResult validate( a.dynamicClass()); List ann = validateReferences(refs, scope, a.query().getPrefixMapping(), originalText); + ann.addAll(UnusedVariableCheck.check(a.query(), originalText)); return new SparqlValidationResult(originalText, plan, ann); } diff --git a/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlValidationApi.java b/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlValidationApi.java index f031dfc3..7a3b147d 100644 --- a/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlValidationApi.java +++ b/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlValidationApi.java @@ -156,7 +156,8 @@ public SparqlValidationResult validateSparql( } /** - * Performs a schema-independent syntax check, returning only {@code SYNTAX_ERROR} annotations. + * Performs a schema-independent check: syntax, plus — for inputs that parse as a query — the + * (equally schema-independent) unused-variable analysis of {@link UnusedVariableCheck}. * *

Used as a fallback when no schema can be resolved (for example a {@code # [endpoint=...]} is * briefly unreachable) so that a cell containing broken SPARQL still gets squiggles rather than @@ -165,8 +166,9 @@ public SparqlValidationResult validateSparql( * {@code cim:} and friends without an explicit {@code PREFIX} line is not mis-reported as a * syntax error. * - * @return a result whose annotations are empty when the input parses as either a query or an - * update, or hold a single {@code SYNTAX_ERROR} when neither parser accepts it + * @return a result whose annotations are empty when the input parses cleanly, hold {@code + * PROJECTED_VARIABLE_UNBOUND}/{@code UNUSED_VARIABLE} warnings for a query with unused + * variables, or hold a single {@code SYNTAX_ERROR} when neither parser accepts the input */ public static SparqlValidationResult checkSyntaxOnly(String input) { Objects.requireNonNull(input, "input"); @@ -174,8 +176,15 @@ public static SparqlValidationResult checkSyntaxOnly(String input) { SparqlQueryAnalyzer analyzer = new SparqlQueryAnalyzer(); InvalidQueryException queryError; try { - analyzer.parse(inj.text()); - return new SparqlValidationResult(input, null, List.of()); + Query query = analyzer.parse(inj.text()); + var warnings = UnusedVariableCheck.check(query, inj.text()); + if (warnings.isEmpty()) { + return new SparqlValidationResult(input, null, List.of()); + } + SparqlValidationResult raw = new SparqlValidationResult(inj.text(), null, warnings); + return inj.injectedLineCount() > 0 + ? adjustLineNumbers(raw, input, inj.injectedLineCount()) + : new SparqlValidationResult(input, null, warnings); } catch (InvalidQueryException e) { queryError = e; // remember the query error; it is more informative than the update one } @@ -240,7 +249,7 @@ public static ShaclValidationResult checkShaclSyntaxOnly( var extractor = new ShaclSparqlExtractor(); var embeddedResults = new ArrayList(); for (EmbeddedSparql q : extractor.extract(shapesGraph)) { - SparqlValidationResult r = checkSyntaxOnly(q.renderedQuery()); + SparqlValidationResult r = suppressShaclPreboundVariables(checkSyntaxOnly(q.renderedQuery())); embeddedResults.add(new ShaclEmbeddedQueryResult(q, r)); } // The vocabulary-typo check is schema-independent (it consults only the bundled W3C @@ -776,13 +785,44 @@ private ShaclValidationResult validateShacl(Graph shapesGraph, ValidationScope s ? Map.of() : Map.of(org.apache.jena.sparql.core.Var.alloc("this"), q.targetClasses()); var r = validator.validate(renderedQueryWithPath(q), scope, hints); - r = suppressImpliedType(r); + r = suppressShaclPreboundVariables(suppressImpliedType(r)); embeddedResults.add(new ShaclEmbeddedQueryResult(q, r)); } return new ShaclValidationResult(shapeAnnotations, embeddedResults); } + /** + * Variables the SHACL engine pre-binds in embedded SPARQL constraints and validators ({@code + * $this}, {@code ?value}, …), plus the {@code $PATH} placeholder. They are legitimately projected + * or referenced without being bound in the query body, so the unused-variable warnings must not + * fire for them. + */ + private static final Set SHACL_PREBOUND_VARIABLES = + Set.of("this", "value", "PATH", "shapesGraph", "currentShape"); + + /** + * Removes {@link SparqlValidationCode#PROJECTED_VARIABLE_UNBOUND} / {@link + * SparqlValidationCode#UNUSED_VARIABLE} annotations about SHACL pre-bound variables from an + * embedded-SPARQL result — e.g. {@code SELECT $this WHERE { FILTER NOT EXISTS { ... } }} is a + * perfectly valid constraint because the engine binds {@code $this} before evaluation. + */ + private static SparqlValidationResult suppressShaclPreboundVariables(SparqlValidationResult r) { + var filtered = + r.annotations().stream() + .filter( + a -> + !((a.code() == SparqlValidationCode.PROJECTED_VARIABLE_UNBOUND + || a.code() == SparqlValidationCode.UNUSED_VARIABLE) + && a.term() != null + && a.term().isVariable() + && SHACL_PREBOUND_VARIABLES.contains(a.term().getName()))) + .toList(); + return filtered.size() == r.annotations().size() + ? r + : new SparqlValidationResult(r.query(), r.queryPlan(), filtered); + } + /** * Removes {@link SparqlValidationCode#QUERY_IMPLIED_TYPE} annotations from an embedded-SPARQL * result before it is stored in the SHACL validation report. diff --git a/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlValidationCode.java b/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlValidationCode.java index 2dbaa6e1..5386b49f 100644 --- a/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlValidationCode.java +++ b/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlValidationCode.java @@ -78,5 +78,17 @@ public enum SparqlValidationCode { * CIM {@code cims:multiplicity} — e.g. requiring more values than the schema's upper bound * allows, so the constraint can never be satisfied against conformant data. */ - CARDINALITY_INCOMPATIBLE_WITH_MULTIPLICITY + CARDINALITY_INCOMPATIBLE_WITH_MULTIPLICITY, + /** + * A variable in the result surface (explicit {@code SELECT} list, {@code CONSTRUCT} template, + * {@code DESCRIBE} list) that appears nowhere in the query body ({@code WHERE}/{@code + * BIND}/{@code VALUES}) — it is unbound in every result, almost always a typo (e.g. {@code ?nmae} + * vs {@code ?name}). + */ + PROJECTED_VARIABLE_UNBOUND, + /** + * A variable bound in a triple pattern, {@code BIND} or {@code VALUES} that is never projected, + * filtered on, ordered/grouped by, or otherwise reused — a typo or a leftover from a query edit. + */ + UNUSED_VARIABLE } diff --git a/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/UnusedVariableCheck.java b/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/UnusedVariableCheck.java new file mode 100644 index 00000000..311fda39 --- /dev/null +++ b/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/UnusedVariableCheck.java @@ -0,0 +1,461 @@ +/* + * Copyright (c) 2026 SOPTIM AG + * + * 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. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +package de.soptim.opencgmes.cimvocabcheck.core; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.apache.jena.graph.Node; +import org.apache.jena.graph.Triple; +import org.apache.jena.query.Query; +import org.apache.jena.query.SortCondition; +import org.apache.jena.sparql.core.Quad; +import org.apache.jena.sparql.core.TriplePath; +import org.apache.jena.sparql.core.Var; +import org.apache.jena.sparql.core.VarExprList; +import org.apache.jena.sparql.expr.Expr; +import org.apache.jena.sparql.expr.ExprAggregator; +import org.apache.jena.sparql.expr.ExprFunction; +import org.apache.jena.sparql.expr.ExprFunctionOp; +import org.apache.jena.sparql.expr.ExprList; +import org.apache.jena.sparql.expr.ExprVar; +import org.apache.jena.sparql.expr.NodeValue; +import org.apache.jena.sparql.syntax.Element; +import org.apache.jena.sparql.syntax.ElementAntiJoin; +import org.apache.jena.sparql.syntax.ElementAssign; +import org.apache.jena.sparql.syntax.ElementBind; +import org.apache.jena.sparql.syntax.ElementData; +import org.apache.jena.sparql.syntax.ElementExists; +import org.apache.jena.sparql.syntax.ElementFilter; +import org.apache.jena.sparql.syntax.ElementGroup; +import org.apache.jena.sparql.syntax.ElementLateral; +import org.apache.jena.sparql.syntax.ElementMinus; +import org.apache.jena.sparql.syntax.ElementNamedGraph; +import org.apache.jena.sparql.syntax.ElementNotExists; +import org.apache.jena.sparql.syntax.ElementOptional; +import org.apache.jena.sparql.syntax.ElementPathBlock; +import org.apache.jena.sparql.syntax.ElementSemiJoin; +import org.apache.jena.sparql.syntax.ElementService; +import org.apache.jena.sparql.syntax.ElementSubQuery; +import org.apache.jena.sparql.syntax.ElementTriplesBlock; +import org.apache.jena.sparql.syntax.ElementUnion; +import org.apache.jena.sparql.syntax.Template; + +/** + * Schema-independent check for variables that are declared but never meaningfully used in a SPARQL + * query. Two cases are reported, both as {@code WARN}: + * + *

    + *
  • {@link SparqlValidationCode#PROJECTED_VARIABLE_UNBOUND} — a variable in the result surface + * (explicit {@code SELECT} list, {@code CONSTRUCT} template, {@code DESCRIBE} list) that does + * not appear anywhere in the query body ({@code WHERE} / {@code BIND} / {@code VALUES}). Such + * a variable is unbound in every result row — almost always a typo (e.g. {@code ?nmae} vs + * {@code ?name}). + *
  • {@link SparqlValidationCode#UNUSED_VARIABLE} — a variable bound in a triple pattern, {@code + * BIND} or {@code VALUES} that occurs exactly once in its query scope: never projected, + * filtered on, ordered/grouped by, or otherwise reused. A typo or a leftover from an edit; a + * deliberately unused pattern variable is idiomatically a blank node. + *
+ * + *

The check works on the parsed syntax tree (not the algebra) so parser-allocated variables — + * blank-node labels, property-path intermediates — never surface. Deliberate design decisions to + * keep the check quiet on idiomatic SPARQL: + * + *

    + *
  • {@code SELECT *} / {@code DESCRIBE *} project every pattern variable, and {@code ASK} + * bodies are pure existence tests — no {@code UNUSED_VARIABLE} is reported for them. + *
  • Occurrences inside {@code EXISTS} / {@code NOT EXISTS} / {@code MINUS} bodies are + * existence-test occurrences: they count as uses of outer variables, but a variable + * occurring only there is not reported. + *
  • Variable predicates ({@code ?s ?p ?o}), {@code GRAPH ?g} / {@code SERVICE ?s} names and + * sub-{@code SELECT} projections joined into the outer scope have no blank-node equivalent — + * a single occurrence there is not reported. + *
  • Each ({@code SELECT}) scope is analyzed independently: a sub-query variable that is not + * projected is invisible to — and independent of — the outer scope. + *
+ * + *

Queries containing syntax forms this walker does not know (e.g. future Jena extensions) are + * skipped entirely rather than risking a false positive. SPARQL Update requests are out of scope: + * they have no projection, and an unbound variable in an INSERT/DELETE template is a different + * class of problem. + */ +public final class UnusedVariableCheck { + + private UnusedVariableCheck() {} + + /** Runs the check on a parsed query; {@code originalText} is used only for source locations. */ + public static List check(Query query, String originalText) { + return check(query, originalText, Set.of()); + } + + /** + * Variant with an exemption list: variables whose name is in {@code exemptVariables} are never + * reported. Used for SHACL embedded SPARQL, where {@code $this}, {@code ?value} etc. are + * pre-bound by the SHACL engine and legitimately projected without appearing in the body. + */ + public static List check( + Query query, String originalText, Set exemptVariables) { + var out = new ArrayList(); + new Walker(originalText, exemptVariables, out).analyzeQuery(query); + return out; + } + + /** How a variable occurrence participates in the query. */ + private enum Kind { + /** Subject or object of a triple / property-path pattern — blank-node replaceable. */ + PATTERN, + /** Predicate of a triple pattern — not blank-node replaceable, exempt from UNUSED_VARIABLE. */ + PATTERN_PREDICATE, + /** Target of {@code BIND} / {@code LET} / {@code UNFOLD}. */ + BIND, + /** Declared by a {@code VALUES} block. */ + VALUES, + /** {@code GRAPH ?g} / {@code SERVICE ?s} name — idiomatic wildcard, exempt. */ + GRAPH_NAME, + /** Projected out of a sub-{@code SELECT} into this scope — exempt. */ + SUBQUERY_PROJECTION, + /** Consumed: expression, projection, solution modifier, template. */ + USAGE + } + + /** Occurrence statistics for one variable within one query scope. */ + private static final class Occurrences { + int total; + Kind firstKind; + boolean firstExistential; + } + + /** Per-{@code SELECT}-scope state; sub-queries get a fresh scope. */ + private static final class Scope { + final Map vars = new LinkedHashMap<>(); + + /** Variables occurring anywhere in the WHERE tree (patterns, expressions, EXISTS bodies). */ + final Set bodyMentioned = new LinkedHashSet<>(); + + /** Variables defined by an {@code (expr AS ?v)} in the projection or GROUP BY. */ + final Set exprDefined = new LinkedHashSet<>(); + + /** Set when an unknown syntax form is encountered — suppresses all findings. */ + boolean unsupported; + } + + private static final class Walker { + private final String text; + private final Set exempt; + private final List out; + + Walker(String text, Set exempt, List out) { + this.text = text; + this.exempt = exempt; + this.out = out; + } + + void analyzeQuery(Query query) { + var scope = new Scope(); + walkElement(query.getQueryPattern(), scope, false); + + // Solution modifiers: uses, but not part of the query body for the unbound check. + VarExprList groupBy = query.getGroupBy(); + if (groupBy != null) { + for (Var v : groupBy.getVars()) { + Expr e = groupBy.getExpr(v); + if (e == null) { + record(scope, v, Kind.USAGE, false, false); + } else { + scope.exprDefined.add(v); + walkExpr(e, scope, false, false); + } + } + } + for (Expr e : query.getHavingExprs()) { + walkExpr(e, scope, false, false); + } + if (query.getOrderBy() != null) { + for (SortCondition sc : query.getOrderBy()) { + walkExpr(sc.getExpression(), scope, false, false); + } + } + // A trailing query-level VALUES binds like the body. + if (query.hasValues()) { + for (Var v : query.getValuesVariables()) { + record(scope, v, Kind.VALUES, false, true); + } + } + + // Result surface: explicitly projected / template / described variables, with the phrase + // used in the PROJECTED_VARIABLE_UNBOUND message. + var resultSurface = new LinkedHashMap(); + boolean star = query.isQueryResultStar(); + if (query.isSelectType() && !star) { + VarExprList project = query.getProject(); + for (Var v : project.getVars()) { + Expr e = project.getExpr(v); + if (e == null) { + resultSurface.put(v, "projected"); + record(scope, v, Kind.USAGE, false, false); + } else { + scope.exprDefined.add(v); + walkExpr(e, scope, false, false); + } + } + } else if (query.isConstructType()) { + Template template = query.getConstructTemplate(); + if (template != null) { + for (Quad quad : template.getQuads()) { + recordTemplateNode(quad.getGraph(), scope, resultSurface); + recordTemplateNode(quad.getSubject(), scope, resultSurface); + recordTemplateNode(quad.getPredicate(), scope, resultSurface); + recordTemplateNode(quad.getObject(), scope, resultSurface); + } + } + } else if (query.isDescribeType() && !star) { + for (Var v : query.getProjectVars()) { + resultSurface.put(v, "described"); + record(scope, v, Kind.USAGE, false, false); + } + } + + if (scope.unsupported) { + return; + } + + // Case 1: in the result surface but nowhere in the query body. + for (var entry : resultSurface.entrySet()) { + Var v = entry.getKey(); + if (exempt.contains(v.getName()) + || scope.bodyMentioned.contains(v) + || scope.exprDefined.contains(v)) { + continue; + } + out.add( + annotation( + SparqlValidationCode.PROJECTED_VARIABLE_UNBOUND, + v, + "Variable ?" + + v.getName() + + " is " + + entry.getValue() + + " but never bound: it does not appear anywhere in the query body" + + " (WHERE / BIND / VALUES).")); + } + + // Case 2: bound exactly once and never reused. Skipped when every pattern variable is + // implicitly part of the result (SELECT * / DESCRIBE *) or the whole body is an existence + // test (ASK). + if (star || query.isAskType()) { + return; + } + for (var entry : scope.vars.entrySet()) { + Var v = entry.getKey(); + Occurrences o = entry.getValue(); + if (exempt.contains(v.getName()) || o.total != 1 || o.firstExistential) { + continue; + } + String boundBy = + switch (o.firstKind) { + case PATTERN -> "in a triple pattern"; + case BIND -> "by BIND"; + case VALUES -> "by VALUES"; + default -> null; + }; + if (boundBy == null) { + continue; + } + out.add( + annotation( + SparqlValidationCode.UNUSED_VARIABLE, + v, + "Variable ?" + + v.getName() + + " is bound " + + boundBy + + " but never used anywhere else in the query — a typo or a leftover?")); + } + } + + // ---- element traversal ----------------------------------------------------------------- + + private void walkElement(Element el, Scope s, boolean existential) { + switch (el) { + case null -> {} + case ElementGroup g -> g.getElements().forEach(e -> walkElement(e, s, existential)); + case ElementTriplesBlock tb -> + tb.getPattern().getList().forEach(t -> recordTriple(t, s, existential)); + case ElementPathBlock pb -> + pb.getPattern().getList().forEach(tp -> recordTriplePath(tp, s, existential)); + case ElementFilter f -> walkExpr(f.getExpr(), s, existential, true); + case ElementBind b -> { + walkExpr(b.getExpr(), s, existential, true); + record(s, b.getVar(), Kind.BIND, existential, true); + } + case ElementAssign a -> { + walkExpr(a.getExpr(), s, existential, true); + record(s, a.getVar(), Kind.BIND, existential, true); + } + case ElementData d -> + d.getVars().forEach(v -> record(s, v, Kind.VALUES, existential, true)); + case ElementUnion u -> u.getElements().forEach(e -> walkElement(e, s, existential)); + case ElementOptional o -> walkElement(o.getOptionalElement(), s, existential); + case ElementLateral l -> walkElement(l.getLateralElement(), s, existential); + case ElementNamedGraph g -> { + recordGraphName(g.getGraphNameNode(), s, existential); + walkElement(g.getElement(), s, existential); + } + case ElementService sv -> { + recordGraphName(sv.getServiceNode(), s, existential); + walkElement(sv.getElement(), s, existential); + } + case ElementMinus m -> walkElement(m.getMinusElement(), s, true); + case ElementExists e -> walkElement(e.getElement(), s, true); + case ElementNotExists e -> walkElement(e.getElement(), s, true); + case ElementSemiJoin j -> walkElement(j.getSubElement(), s, true); + case ElementAntiJoin j -> walkElement(j.getSubElement(), s, true); + case ElementSubQuery sq -> { + analyzeQuery(sq.getQuery()); + for (Var v : projectedVars(sq.getQuery(), s)) { + record(s, v, Kind.SUBQUERY_PROJECTION, existential, true); + } + } + default -> s.unsupported = true; + } + } + + /** Variables a sub-query makes visible to the enclosing scope. */ + private static List projectedVars(Query subQuery, Scope s) { + try { + return subQuery.getProjectVars(); + } catch (RuntimeException e) { + s.unsupported = true; + return List.of(); + } + } + + private void recordTriple(Triple t, Scope s, boolean existential) { + recordPatternNode(t.getSubject(), s, Kind.PATTERN, existential); + recordPatternNode(t.getPredicate(), s, Kind.PATTERN_PREDICATE, existential); + recordPatternNode(t.getObject(), s, Kind.PATTERN, existential); + } + + private void recordTriplePath(TriplePath tp, Scope s, boolean existential) { + if (tp.isTriple()) { + recordTriple(tp.asTriple(), s, existential); + return; + } + recordPatternNode(tp.getSubject(), s, Kind.PATTERN, existential); + recordPatternNode(tp.getObject(), s, Kind.PATTERN, existential); + } + + private void recordPatternNode(Node n, Scope s, Kind kind, boolean existential) { + if (n == null) { + return; + } + if (n.isTripleTerm()) { // RDF-star quoted triple + recordTriple(n.getTriple(), s, existential); + } else if (n.isVariable()) { + record(s, Var.alloc(n.getName()), kind, existential, true); + } + } + + private void recordGraphName(Node n, Scope s, boolean existential) { + if (n != null && n.isVariable()) { + record(s, Var.alloc(n.getName()), Kind.GRAPH_NAME, existential, true); + } + } + + private void recordTemplateNode(Node n, Scope s, Map resultSurface) { + if (n == null) { + return; + } + if (n.isTripleTerm()) { + Triple t = n.getTriple(); + recordTemplateNode(t.getSubject(), s, resultSurface); + recordTemplateNode(t.getPredicate(), s, resultSurface); + recordTemplateNode(t.getObject(), s, resultSurface); + } else if (n.isVariable()) { + Var v = Var.alloc(n.getName()); + if (Var.isNamedVarName(v.getName())) { + resultSurface.putIfAbsent(v, "used in the CONSTRUCT template"); + } + record(s, v, Kind.USAGE, false, false); + } + } + + // ---- expression traversal ---------------------------------------------------------------- + + private void walkExpr(Expr e, Scope s, boolean existential, boolean inBody) { + switch (e) { + case null -> {} + case ExprVar v -> record(s, v.asVar(), Kind.USAGE, existential, inBody); + case ExprAggregator agg -> { + ExprList args = agg.getAggregator().getExprList(); + if (args != null) { + args.forEach(a -> walkExpr(a, s, existential, inBody)); + } + } + case ExprFunctionOp op -> { + // EXISTS / NOT EXISTS: an existence test over the query body. + Element pattern = op.getElement(); + if (pattern == null) { + s.unsupported = true; + } else { + walkElement(pattern, s, true); + } + } + case ExprFunction f -> f.getArgs().forEach(a -> walkExpr(a, s, existential, inBody)); + case NodeValue ignored -> {} + default -> s.unsupported = true; + } + } + + // ---- bookkeeping --------------------------------------------------------------------------- + + private void record(Scope s, Var v, Kind kind, boolean existential, boolean inBody) { + if (!Var.isNamedVarName(v.getName())) { + return; // parser-allocated variable (blank-node label, path intermediate) + } + Occurrences o = s.vars.computeIfAbsent(v, k -> new Occurrences()); + if (o.total == 0) { + o.firstKind = kind; + o.firstExistential = existential; + } + o.total++; + if (inBody) { + s.bodyMentioned.add(v); + } + } + + private SparqlValidationAnnotation annotation( + SparqlValidationCode code, Var v, String message) { + var loc = SourceLocator.locateVariable(text, v.getName()); + return new SparqlValidationAnnotation( + SparqlValidationSeverity.WARN, + loc.line(), + loc.column(), + message, + code, + v, + List.of(), + List.of(), + null); + } + } +} diff --git a/cimvocabcheck/core/src/test/java/de/soptim/opencgmes/cimvocabcheck/core/UnusedVariableCheckTest.java b/cimvocabcheck/core/src/test/java/de/soptim/opencgmes/cimvocabcheck/core/UnusedVariableCheckTest.java new file mode 100644 index 00000000..68afe2e0 --- /dev/null +++ b/cimvocabcheck/core/src/test/java/de/soptim/opencgmes/cimvocabcheck/core/UnusedVariableCheckTest.java @@ -0,0 +1,331 @@ +/* + * Copyright (c) 2026 SOPTIM AG + * + * 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. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +package de.soptim.opencgmes.cimvocabcheck.core; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.List; +import org.apache.jena.rdf.model.ModelFactory; +import org.apache.jena.riot.Lang; +import org.apache.jena.riot.RDFParser; +import org.junit.Test; + +/** + * Tests for the unused-variable warnings {@code PROJECTED_VARIABLE_UNBOUND} and {@code + * UNUSED_VARIABLE}. The check is schema-independent, so it is exercised through {@link + * SparqlValidationApi#checkSyntaxOnly(String)}. + */ +public class UnusedVariableCheckTest { + + private static List warnings(String query) { + return SparqlValidationApi.checkSyntaxOnly(query).annotations(); + } + + private static List ofCode( + List annotations, SparqlValidationCode code) { + return annotations.stream().filter(a -> a.code() == code).toList(); + } + + // ---- case 1: projected but unbound ------------------------------------------------------- + + @Test + public void projectedButUnboundVariableIsReported() { + var ann = + warnings( + """ + SELECT ?s ?nmae WHERE { + ?s cim:name ?name . + FILTER(?name != "") + } + """); + assertEquals(1, ann.size()); + var a = ann.get(0); + assertEquals(SparqlValidationCode.PROJECTED_VARIABLE_UNBOUND, a.code()); + assertEquals(SparqlValidationSeverity.WARN, a.severity()); + assertTrue(a.message().contains("?nmae")); + assertEquals("nmae", a.term().getName()); + // Located at the ?nmae token in the SELECT clause (line 1, after "SELECT ?s "). + assertEquals(Integer.valueOf(1), a.line()); + assertEquals(Integer.valueOf(11), a.column()); + } + + @Test + public void projectionExpressionArgumentsCountAsUsage() { + var ann = + warnings( + """ + SELECT ?s (STRLEN(?name) AS ?len) WHERE { + ?s cim:name ?name . + } + """); + assertTrue(ann.isEmpty()); + } + + @Test + public void orderByAloneDoesNotBindAProjectedVariable() { + var ann = + warnings("SELECT ?s ?x WHERE { ?s a cim:ACLineSegment . FILTER(?s != ?x) } ORDER BY ?x"); + // ?x is used in a FILTER inside the body — conservatively counted as appearing in the + // query body, so no PROJECTED_VARIABLE_UNBOUND is raised for it. + assertTrue(ann.isEmpty()); + } + + @Test + public void constructTemplateVariableNeverBoundIsReported() { + var ann = + warnings( + """ + CONSTRUCT { ?s cim:IdentifiedObject.name ?nmae } WHERE { + ?s cim:name ?name . + FILTER(?name != "") + } + """); + assertEquals(1, ann.size()); + assertEquals(SparqlValidationCode.PROJECTED_VARIABLE_UNBOUND, ann.get(0).code()); + assertTrue(ann.get(0).message().contains("CONSTRUCT template")); + } + + @Test + public void describeVariableNeverBoundIsReported() { + var ann = warnings("DESCRIBE ?x WHERE { ?s a cim:ACLineSegment . FILTER(?s != ) }"); + assertEquals(1, ann.size()); + assertEquals(SparqlValidationCode.PROJECTED_VARIABLE_UNBOUND, ann.get(0).code()); + assertTrue(ann.get(0).message().contains("?x")); + } + + @Test + public void trailingValuesBlockBindsProjectedVariable() { + var ann = + warnings( + "SELECT ?s ?v WHERE { ?s a cim:ACLineSegment . FILTER(?s != ?v) } VALUES ?v { 1 2 }"); + assertTrue(ann.isEmpty()); + } + + // ---- case 2: bound but unreferenced ------------------------------------------------------- + + @Test + public void boundButUnreferencedVariableIsReported() { + var ann = + warnings( + """ + SELECT ?s WHERE { + ?s a cim:ACLineSegment . + ?s cim:name ?name . + } + """); + assertEquals(1, ann.size()); + var a = ann.get(0); + assertEquals(SparqlValidationCode.UNUSED_VARIABLE, a.code()); + assertEquals(SparqlValidationSeverity.WARN, a.severity()); + assertTrue(a.message().contains("?name")); + assertEquals(Integer.valueOf(3), a.line()); + } + + @Test + public void unusedBindTargetIsReported() { + var ann = warnings("SELECT ?s WHERE { ?s a cim:ACLineSegment . BIND(1 AS ?x) }"); + assertEquals(1, ann.size()); + assertEquals(SparqlValidationCode.UNUSED_VARIABLE, ann.get(0).code()); + assertTrue(ann.get(0).message().contains("BIND")); + } + + @Test + public void unusedValuesVariableIsReported() { + var ann = warnings("SELECT ?s WHERE { ?s a cim:ACLineSegment . VALUES ?v { 1 } }"); + assertEquals(1, ann.size()); + assertEquals(SparqlValidationCode.UNUSED_VARIABLE, ann.get(0).code()); + assertTrue(ann.get(0).message().contains("VALUES")); + } + + @Test + public void filterUsageCountsAsUse() { + var ann = warnings("SELECT ?s WHERE { ?s cim:name ?name . FILTER(STRLEN(?name) > 0) }"); + assertTrue(ann.isEmpty()); + } + + @Test + public void orderByCountsAsUse() { + var ann = warnings("SELECT ?s WHERE { ?s cim:name ?name } ORDER BY ?name"); + assertTrue(ann.isEmpty()); + } + + @Test + public void aggregateArgumentCountsAsUse() { + var ann = warnings("SELECT ?type (COUNT(?s) AS ?n) WHERE { ?s a ?type } GROUP BY ?type"); + assertTrue(ann.isEmpty()); + } + + @Test + public void selectStarProjectsEverything() { + var ann = warnings("SELECT * WHERE { ?s cim:name ?name }"); + assertTrue(ann.isEmpty()); + } + + @Test + public void askBodyIsAnExistenceTest() { + var ann = warnings("ASK { ?s cim:name ?name }"); + assertTrue(ann.isEmpty()); + } + + @Test + public void notExistsBodyIsAnExistenceTest() { + var ann = + warnings( + "SELECT ?s WHERE { ?s a cim:ACLineSegment . FILTER NOT EXISTS { ?s cim:name ?any } }"); + assertTrue(ann.isEmpty()); + } + + @Test + public void minusBodyIsAnExistenceTest() { + var ann = warnings("SELECT ?s WHERE { ?s a cim:ACLineSegment MINUS { ?s cim:name ?x } }"); + assertTrue(ann.isEmpty()); + } + + @Test + public void variablePredicateIsNotReported() { + // ?p cannot be replaced by a blank node — a single-use predicate variable is not flagged + // (the dynamic-predicate notice covers that situation in schema-aware validation). + var ann = warnings("SELECT ?s WHERE { ?s ?p }"); + assertTrue(ann.isEmpty()); + } + + @Test + public void graphNameWildcardIsNotReported() { + var ann = warnings("SELECT ?s WHERE { GRAPH ?g { ?s a cim:ACLineSegment } }"); + assertTrue(ann.isEmpty()); + } + + @Test + public void variableUsedTwiceInOnePatternIsAJoin() { + var ann = warnings("SELECT ?s WHERE { ?s cim:name ?s }"); + assertTrue(ann.isEmpty()); + } + + // ---- sub-query scoping -------------------------------------------------------------------- + + @Test + public void subQueryScopeIsAnalyzedIndependently() { + var ann = + warnings( + """ + SELECT ?s WHERE { + { SELECT ?s WHERE { ?s cim:name ?x } } + } + """); + // ?x is unused inside the sub-query; the sub-query's projected ?s joining the outer scope + // only once is fine. + assertEquals(1, ann.size()); + assertEquals(SparqlValidationCode.UNUSED_VARIABLE, ann.get(0).code()); + assertTrue(ann.get(0).message().contains("?x")); + } + + @Test + public void innerScopeVariableDoesNotSatisfyOuterProjection() { + var ann = + warnings( + """ + SELECT ?s ?x WHERE { + { SELECT ?s WHERE { ?s cim:name ?x . FILTER(?x != "") } } + } + """); + // The inner ?x is not projected out of the sub-query, so the outer projected ?x is unbound. + var unbound = ofCode(ann, SparqlValidationCode.PROJECTED_VARIABLE_UNBOUND); + assertEquals(1, unbound.size()); + assertTrue(unbound.get(0).message().contains("?x")); + } + + // ---- updates are out of scope -------------------------------------------------------------- + + @Test + public void updatesAreNotChecked() { + var ann = warnings("DELETE { ?s ?p ?o } WHERE { ?s ?p ?o }"); + assertTrue(ann.isEmpty()); + } + + // ---- strictness interaction ----------------------------------------------------------------- + + @Test + public void permissiveStrictnessSuppressesTheWarnings() { + var ann = warnings("SELECT ?s ?nmae WHERE { ?s cim:name ?name . FILTER(?name != \"\") }"); + assertEquals(1, ann.size()); + assertTrue(StrictnessLevel.PERMISSIVE.apply(ann).isEmpty()); + } + + // ---- SHACL embedded SPARQL: pre-bound variables ---------------------------------------------- + + private static org.apache.jena.graph.Graph turtle(String ttl) { + var m = ModelFactory.createDefaultModel(); + RDFParser.fromString(ttl, Lang.TURTLE).parse(m); + return m.getGraph(); + } + + @Test + public void shaclPreboundVariablesAreExempt() { + // $this is projected without appearing in a binding position — legal in SHACL because the + // engine pre-binds it; must not be reported. + var g = + turtle( + """ + @prefix sh: . + @prefix cim: . + cim:Shape a sh:NodeShape ; + sh:targetClass cim:ACLineSegment ; + sh:sparql [ + sh:select ""\" + SELECT $this WHERE { + FILTER NOT EXISTS { $this ?n } + } + ""\" ; + ] . + """); + var result = SparqlValidationApi.checkShaclSyntaxOnly(g); + for (var er : result.embeddedResults()) { + assertTrue(er.result().annotations().isEmpty()); + } + } + + @Test + public void shaclEmbeddedQueryWithGenuinelyUnusedVariableIsReported() { + var g = + turtle( + """ + @prefix sh: . + @prefix cim: . + cim:Shape a sh:NodeShape ; + sh:targetClass cim:ACLineSegment ; + sh:sparql [ + sh:select ""\" + SELECT $this ?value WHERE { + $this ?value . + $this ?leftover . + } + ""\" ; + ] . + """); + var result = SparqlValidationApi.checkShaclSyntaxOnly(g); + var all = + result.embeddedResults().stream() + .flatMap(er -> er.result().annotations().stream()) + .toList(); + assertEquals(1, all.size()); + assertEquals(SparqlValidationCode.UNUSED_VARIABLE, all.get(0).code()); + assertTrue(all.get(0).message().contains("?leftover")); + } +} diff --git a/docs/content/cimvocabcheck/validation-checks.md b/docs/content/cimvocabcheck/validation-checks.md index d59b47ae..15f5609d 100644 --- a/docs/content/cimvocabcheck/validation-checks.md +++ b/docs/content/cimvocabcheck/validation-checks.md @@ -35,6 +35,8 @@ map to pass/fail is controlled by [`strictness`](/cimvocabcheck/configuration#st | `INVALID_CARDINALITY` | ERROR | SHACL | `sh:minCount` exceeds `sh:maxCount` on the same property shape | | `INVALID_VALUE_RANGE` | ERROR | SHACL | A value-range constraint is self-contradictory — a lower bound (`sh:minInclusive`/`sh:minExclusive`) exceeds an upper bound (`sh:maxInclusive`/`sh:maxExclusive`) | | `CARDINALITY_INCOMPATIBLE_WITH_MULTIPLICITY` | WARN | SHACL | `sh:minCount`/`sh:maxCount` cannot be satisfied given the property's declared CIM `cims:multiplicity` | +| `PROJECTED_VARIABLE_UNBOUND` | WARN | Variables | A `SELECT`/`DESCRIBE`/`CONSTRUCT`-template variable appears nowhere in the query body | +| `UNUSED_VARIABLE` | WARN | Variables | A variable bound in a triple pattern, `BIND`, or `VALUES` is never used anywhere else | :::note The "exists in another profile" hint When a class or property does not exist in the *selected* profiles but **does** exist in another @@ -100,6 +102,35 @@ real CGMES RDFS files), these additional checks run: `rdfs:subClassOf` traversal is transitive and cycle-safe across the union of all profiles in scope. +## Unused-variable checks + +Two schema-independent warnings catch variables that are declared but never meaningfully used — +typically a typo (`?nmae` vs `?name`) or a leftover from a query edit. Because they need no schema, +they also fire in the syntax-only fallback (no config, unreachable endpoint): + +- `PROJECTED_VARIABLE_UNBOUND` — a variable in the result surface (an explicit `SELECT` list, a + `CONSTRUCT` template, a `DESCRIBE` list) that does not appear anywhere in the query body + (`WHERE` / `BIND` / `VALUES`). It is unbound in every result row. +- `UNUSED_VARIABLE` — a variable bound in a triple pattern, `BIND`, or `VALUES` that occurs exactly + once in its scope: never projected, filtered on, ordered/grouped by, or otherwise reused. A + deliberately unused pattern variable is idiomatically written as a blank node (`[]`). + +The check stays quiet on idiomatic SPARQL — no warning is raised for: + +- variables under `SELECT *` / `DESCRIBE *` (everything is projected), or anywhere in an `ASK` + body (a pure existence test); +- single-use variables inside `EXISTS` / `NOT EXISTS` / `MINUS` bodies (existence tests too), + although occurrences there do count as *uses* of outer variables; +- variable predicates (`?s ?p ?o`), `GRAPH ?g` / `SERVICE ?s` names, and sub-`SELECT` projections + joined into the outer scope — none of which can be replaced by a blank node; +- SHACL pre-bound variables in embedded constraint SPARQL (`$this`, `?value`, `$PATH`, + `$shapesGraph`, `$currentShape`), which the SHACL engine binds before evaluation. + +Each `SELECT` scope is analyzed independently, mirroring SPARQL's sub-query semantics. SPARQL +Update requests are not checked. Both findings are `WARN`, so +[`strictness`](/cimvocabcheck/configuration#strictness) `permissive` suppresses them and `strict` +turns them into errors. + :::tip Lenience policy — silent when the schema is silent A semantic check is **skipped** when the schema doesn't carry the information it needs: no `rdfs:domain` → no domain/implied-type check; no `rdfs:range` (or a class range) → no datatype From e32cf044a48be7a282b33eab04bd8801caf8903d Mon Sep 17 00:00:00 2001 From: Jan-Hendrik Spahn Date: Thu, 9 Jul 2026 10:19:48 +0200 Subject: [PATCH 2/2] fix(cimvocabcheck): exempt custom SHACL constraint-component parameters from unused-variable check (GH-23) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit $this/?value/etc. were exempt via a fixed name list, but SHACL-SPARQL constraint components can declare arbitrary pre-bound parameters via sh:parameter [ sh:path ex:foo ] -> $foo (SHACL-SPARQL §5.3). A pattern like `$myList (rdf:rest*)/rdf:first ?path .` was flagged UNUSED_VARIABLE on $myList even though it's legitimately pre-bound, not unused. ShaclSparqlExtractor.collectConstraintParameterNames() now scans the shapes graph for declared sh:parameter local names, and both SHACL suppression call sites (validateShacl, checkShaclSyntaxOnly) merge them with the fixed pre-bound set before filtering. Co-Authored-By: Claude Sonnet 5 --- .../core/SparqlValidationApi.java | 29 ++++++++++++++--- .../core/shacl/ShaclSparqlExtractor.java | 32 +++++++++++++++++++ .../core/UnusedVariableCheckTest.java | 32 +++++++++++++++++++ 3 files changed, 89 insertions(+), 4 deletions(-) diff --git a/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlValidationApi.java b/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlValidationApi.java index 7a3b147d..5313b507 100644 --- a/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlValidationApi.java +++ b/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/SparqlValidationApi.java @@ -247,9 +247,11 @@ public static ShaclValidationResult checkShaclSyntaxOnly( Graph shapesGraph, boolean checkStandardVocabulary) { Objects.requireNonNull(shapesGraph, "shapesGraph"); var extractor = new ShaclSparqlExtractor(); + Set exemptVars = shaclExemptVariableNames(shapesGraph); var embeddedResults = new ArrayList(); for (EmbeddedSparql q : extractor.extract(shapesGraph)) { - SparqlValidationResult r = suppressShaclPreboundVariables(checkSyntaxOnly(q.renderedQuery())); + SparqlValidationResult r = + suppressShaclPreboundVariables(checkSyntaxOnly(q.renderedQuery()), exemptVars); embeddedResults.add(new ShaclEmbeddedQueryResult(q, r)); } // The vocabulary-typo check is schema-independent (it consults only the bundled W3C @@ -778,6 +780,7 @@ private ShaclValidationResult validateShacl(Graph shapesGraph, ValidationScope s // Embedded-SPARQL analysis: sh:select, sh:ask, sh:construct. // For each query, pass $this → sh:targetClass as a subject-type hint so that // domain/range checks fire correctly even when $this has no explicit rdf:type in the query. + Set exemptVars = shaclExemptVariableNames(shapesGraph); var embeddedResults = new ArrayList(); for (EmbeddedSparql q : shaclExtractor.extract(shapesGraph)) { Map> hints = @@ -785,7 +788,7 @@ private ShaclValidationResult validateShacl(Graph shapesGraph, ValidationScope s ? Map.of() : Map.of(org.apache.jena.sparql.core.Var.alloc("this"), q.targetClasses()); var r = validator.validate(renderedQueryWithPath(q), scope, hints); - r = suppressShaclPreboundVariables(suppressImpliedType(r)); + r = suppressShaclPreboundVariables(suppressImpliedType(r), exemptVars); embeddedResults.add(new ShaclEmbeddedQueryResult(q, r)); } @@ -801,13 +804,31 @@ private ShaclValidationResult validateShacl(Graph shapesGraph, ValidationScope s private static final Set SHACL_PREBOUND_VARIABLES = Set.of("this", "value", "PATH", "shapesGraph", "currentShape"); + /** + * Returns the full set of variable names the unused-variable check must treat as pre-bound for + * {@code shapesGraph}: the fixed {@link #SHACL_PREBOUND_VARIABLES} plus every custom {@code + * sh:ConstraintComponent} parameter name declared in the graph itself (see {@link + * ShaclSparqlExtractor#collectConstraintParameterNames(Graph)}). Custom parameters are just as + * pre-bound as {@code $this} — a component declaring {@code sh:parameter [ sh:path ex:myList ]} + * legitimately references {@code $myList} in its SPARQL body without binding it there. + */ + private static Set shaclExemptVariableNames(Graph shapesGraph) { + var names = new LinkedHashSet<>(SHACL_PREBOUND_VARIABLES); + names.addAll(ShaclSparqlExtractor.collectConstraintParameterNames(shapesGraph)); + return names; + } + /** * Removes {@link SparqlValidationCode#PROJECTED_VARIABLE_UNBOUND} / {@link * SparqlValidationCode#UNUSED_VARIABLE} annotations about SHACL pre-bound variables from an * embedded-SPARQL result — e.g. {@code SELECT $this WHERE { FILTER NOT EXISTS { ... } }} is a * perfectly valid constraint because the engine binds {@code $this} before evaluation. + * + * @param exemptNames the pre-bound variable names for the enclosing shapes graph, from {@link + * #shaclExemptVariableNames(Graph)} */ - private static SparqlValidationResult suppressShaclPreboundVariables(SparqlValidationResult r) { + private static SparqlValidationResult suppressShaclPreboundVariables( + SparqlValidationResult r, Set exemptNames) { var filtered = r.annotations().stream() .filter( @@ -816,7 +837,7 @@ private static SparqlValidationResult suppressShaclPreboundVariables(SparqlValid || a.code() == SparqlValidationCode.UNUSED_VARIABLE) && a.term() != null && a.term().isVariable() - && SHACL_PREBOUND_VARIABLES.contains(a.term().getName()))) + && exemptNames.contains(a.term().getName()))) .toList(); return filtered.size() == r.annotations().size() ? r diff --git a/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/shacl/ShaclSparqlExtractor.java b/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/shacl/ShaclSparqlExtractor.java index 55baac34..77d59f98 100644 --- a/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/shacl/ShaclSparqlExtractor.java +++ b/cimvocabcheck/core/src/main/java/de/soptim/opencgmes/cimvocabcheck/core/shacl/ShaclSparqlExtractor.java @@ -82,6 +82,38 @@ public List extract(Graph shapesGraph) { return List.copyOf(out); } + /** + * Collects the local names of custom {@code sh:ConstraintComponent} parameters declared in {@code + * shapesGraph} via {@code sh:parameter [ sh:path ex:foo ]}, e.g. {@code "foo"}. + * + *

Per SHACL-SPARQL (§5.3), the SPARQL variable a constraint component's query body pre-binds + * for such a parameter is named after the local name of its {@code sh:path} — exactly like the + * built-in {@code $this} / {@code ?value}, but with an author-chosen name. A property shape's + * {@code $myList} is therefore a legitimate pre-bound reference, not an unbound/unused variable, + * whenever {@code myList} matches a declared parameter's local name anywhere in the shapes graph. + */ + public static Set collectConstraintParameterNames(Graph shapesGraph) { + var names = new LinkedHashSet(); + Iterator it = shapesGraph.find(Node.ANY, Shacl.PARAMETER, Node.ANY); + try { + while (it.hasNext()) { + Node path = singleObject(shapesGraph, it.next().getObject(), Shacl.PATH); + if (path != null && path.isURI()) { + names.add(localName(path.getURI())); + } + } + } finally { + JenaIterators.closeQuietly(it); + } + return names; + } + + /** Returns the local name of a URI (the part after the last {@code #} or {@code /}). */ + private static String localName(String uri) { + int sep = Math.max(uri.lastIndexOf('#'), uri.lastIndexOf('/')); + return sep >= 0 ? uri.substring(sep + 1) : uri; + } + private static void collectQueries( Graph g, Node predicate, diff --git a/cimvocabcheck/core/src/test/java/de/soptim/opencgmes/cimvocabcheck/core/UnusedVariableCheckTest.java b/cimvocabcheck/core/src/test/java/de/soptim/opencgmes/cimvocabcheck/core/UnusedVariableCheckTest.java index 68afe2e0..c936b3b7 100644 --- a/cimvocabcheck/core/src/test/java/de/soptim/opencgmes/cimvocabcheck/core/UnusedVariableCheckTest.java +++ b/cimvocabcheck/core/src/test/java/de/soptim/opencgmes/cimvocabcheck/core/UnusedVariableCheckTest.java @@ -328,4 +328,36 @@ public void shaclEmbeddedQueryWithGenuinelyUnusedVariableIsReported() { assertEquals(SparqlValidationCode.UNUSED_VARIABLE, all.get(0).code()); assertTrue(all.get(0).message().contains("?leftover")); } + + @Test + public void customConstraintComponentParameterIsExempt() { + // ex:myList is a custom sh:ConstraintComponent parameter (sh:parameter [ sh:path ex:myList ]), + // so $myList is pre-bound by the SHACL engine — same as $this — even though it appears exactly + // once in the query body, as the subject of a property-path pattern that unpacks the list. + var g = + turtle( + """ + @prefix sh: . + @prefix ex: . + @prefix rdf: . + ex:ListContainsComponent + a sh:ConstraintComponent ; + sh:parameter [ sh:path ex:myList ] ; + sh:validator [ + a sh:SPARQLSelectValidator ; + sh:select ""\" + SELECT $this ?path WHERE { + $myList (rdf:rest*)/rdf:first ?path . + FILTER (?path = $this) + } + ""\" ; + ] . + """); + var result = SparqlValidationApi.checkShaclSyntaxOnly(g); + var all = + result.embeddedResults().stream() + .flatMap(er -> er.result().annotations().stream()) + .toList(); + assertTrue(all.isEmpty()); + } }