412 [SPARQL] Validate operand types in expressions#457
Draft
MaillPierre wants to merge 17 commits into
Draft
Conversation
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
MaillPierre
added
the
Refactoring
There was a problem hiding this comment.
Pull request overview
This PR extends the “next query” SPARQL semantic validation layer to validate operand/result types in expressions (notably FILTER/HAVING boolean contexts, numeric-only operators/functions, boolean-only operators, and disallowing IRI ordering comparisons), and introduces/expands an AST visitor pattern (VisitableAst + AstVisitor) to traverse query ASTs consistently for these validations.
Changes:
- Add new semantic validation rules for FILTER/HAVING boolean resolvability and for numeric/boolean/IRI operand-type constraints.
- Refactor/standardize AST traversal by adding
accept(AstVisitor)across many AST nodes and addinggetName()toTermAstfor diagnostics. - Update and expand parser validation tests to cover new type-validation behavior and updated unary argument accessors.
Reviewed changes
Copilot reviewed 143 out of 143 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/fr/inria/corese/core/next/query/impl/parser/SparqlParserValidationTest.java | Adds extensive negative/positive tests for FILTER + operand type validation. |
| src/test/java/fr/inria/corese/core/next/query/impl/parser/SparqlParserUcaseTest.java | Updates unary-argument accessor usage (argument()). |
| src/test/java/fr/inria/corese/core/next/query/impl/parser/SparqlParserStrUuidTest.java | Updates unary-argument accessor usage (argument()). |
| src/test/java/fr/inria/corese/core/next/query/impl/parser/SparqlParserNumericFunctionsTest.java | Adjusts numeric function parsing tests to keep FILTER boolean-valid (comparison wrapping). |
| src/test/java/fr/inria/corese/core/next/query/impl/parser/SparqlParserLcaseTest.java | Updates unary-argument accessor usage (argument()). |
| src/test/java/fr/inria/corese/core/next/query/impl/parser/SparqlParserIriTest.java | Updates unary-argument accessor usage (argument()). |
| src/test/java/fr/inria/corese/core/next/query/impl/parser/SparqlParserHashFunctionsTest.java | Updates unary-argument accessor usage (argument()). |
| src/test/java/fr/inria/corese/core/next/query/impl/parser/SparqlParserFilterTest.java | Updates unary-argument accessor usage and adjusts comparison tests to avoid IRI ordering comparisons. |
| src/test/java/fr/inria/corese/core/next/query/impl/parser/SparqlParserEncodeForUriTest.java | Updates unary-argument accessor usage (argument()). |
| src/test/java/fr/inria/corese/core/next/query/impl/parser/SparqlParserDateTimeFunctionsTest.java | Updates unary-argument accessor usage (argument()) and relies on new getName/visitor. |
| src/test/java/fr/inria/corese/core/next/query/impl/parser/SparqlParserCoalesceTest.java | Updates COALESCE tests to keep FILTER boolean-valid (comparison wrapping). |
| src/test/java/fr/inria/corese/core/next/query/impl/parser/SparqlParserBnodeTest.java | Updates unary-argument accessor usage (argument()). |
| src/test/java/fr/inria/corese/core/next/query/impl/parser/SparqlParserAggregateTest.java | Adds small clarifying helper doc for aggregate extraction in tests. |
| src/test/java/fr/inria/corese/core/next/query/impl/parser/semantic/support/SemanticValidationUtilsTest.java | New unit tests for semantic type-check helpers (boolean/numeric/IRI/unknown). |
| src/main/java/fr/inria/corese/core/sparql/triple/parser/TopExp.java | Renames legacy visitor interface usage to AstVisitable. |
| src/main/java/fr/inria/corese/core/sparql/triple/parser/ASTQuery.java | Renames legacy visitor interface usage to AstVisitable. |
| src/main/java/fr/inria/corese/core/sparql/triple/api/AstVisitable.java | Renames interface from ASTVisitable to AstVisitable. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/SparqlAstToExpression.java | Updates unary constraint accessor from getArgument() to argument(). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/VarAst.java | Adds getName() and visitor accept(...). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/ValuesAst.java | Implements visitor traversal for VALUES. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/ValueMappingAst.java | Implements visitor traversal for VALUES mappings. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/UpdateRequestUnitAst.java | Makes update units visitable. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/UpdateRequestAst.java | Adds visitor entry point for update requests. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/UnionAst.java | Adds visitor traversal for UNION patterns. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/TriplePatternAst.java | Makes triple patterns visitable and traversable. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/TermAst.java | Adds getName() and makes terms visitable for diagnostics/validation traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/SubQueryAst.java | Adds visitor traversal for subqueries in patterns. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/SolutionModifierAst.java | Adds visitor traversal for ORDER/GROUP/HAVING etc. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/ServiceAst.java | Adds visitor traversal for SERVICE patterns. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/SelectQueryAst.java | Adds visitor traversal for SELECT query AST. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/QueryPrologueAst.java | Makes prologue visitable (prefix/base traversal). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/QueryAst.java | Makes QueryAst visitable. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/ProjectionAst.java | Makes projection visitable (variable traversal). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/PrefixDeclarationAst.java | Makes prefix declarations visitable. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/PatternAst.java | Makes patterns visitable; adds visitor accept contract. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/OrderConditionAst.java | Makes ORDER conditions visitable. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/OptionalAst.java | Adds visitor traversal for OPTIONAL patterns. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/MinusAst.java | Adds visitor traversal for MINUS patterns. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/LoadRequestAst.java | Adds visitor traversal for LOAD update operation. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/LiteralAst.java | Adds getName() and visitor accept(...). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/IriAst.java | Adds getName() and visitor accept(...). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/HavingAst.java | Makes HAVING visitable (condition traversal). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/GroupGraphPatternAst.java | Adds visitor traversal for group graph patterns. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/GroupByAst.java | Makes GROUP BY visitable. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/GraphRefAst.java | Makes graph refs visitable (update/Dataset traversal). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/FilterAst.java | Adds visitor traversal for FILTER patterns. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/DescribeQueryAst.java | Adds visitor traversal for DESCRIBE query AST. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/DatasetClauseAst.java | Makes dataset clause visitable. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/ConstructTemplateAst.java | Makes construct template visitable. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/ConstructQueryAst.java | Adds visitor traversal for CONSTRUCT query AST. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/ConstraintAst.java | Makes constraints explicitly visitable (aligned with TermAst changes). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/YearAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/UuidAst.java | Adds getName() and visitor accept(...). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/UnlimitedArgumentsFunctionAst.java | New interface for n-ary functions (arguments list). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/UnaryPlusAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/UnaryMinusAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/UnaryConstraintAst.java | Renames unary accessor to argument(). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/UcaseAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/TzAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/TrinaryRegexAst.java | Adds getName() and visitor traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/TimezoneAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/SubtractAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/SubstrAst.java | Adds getName() and visitor traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/StrUuidAst.java | Adds getName() and visitor accept(...). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/StrStartsAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/StrLenAst.java | Refactors STRLEN into unary constraint class and adds getName(). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/StrLangAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/StrEndsAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/StrDtAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/StrBeforeAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/StrAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/StrAfterAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/Sha512Ast.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/Sha384Ast.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/Sha256Ast.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/Sha1Ast.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/SecondsAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/SameTermAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/RoundAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/ReplaceAst.java | Adds getName() and visitor traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/RandAst.java | Adds getName() and visitor traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/OrAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/NowAst.java | Adds getName() and visitor traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/NotInAst.java | Adds getName() and visitor traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/NotExistsAst.java | Adds getName() and visitor traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/MultiplyAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/MonthAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/MinutesAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/Md5Ast.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/LowerThanAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/LowerOrEqualThanAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/LcaseAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/LangMatchesAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/LangAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/IsLiteralAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/IsIriAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/IsBlankAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/IriFunctionAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/InAst.java | Adds getName() and visitor traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/IfAst.java | Adds getName() and visitor traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/HoursAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/GreaterThanAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/GreaterOrEqualThanAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/FunctionCallAst.java | Adds getName() and visitor traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/FloorAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/ExistsAst.java | Adds getName() and visitor traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/EqualsAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/EncodeForUriAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/DivideAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/DifferentAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/DayAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/DatatypeAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/ContainsAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/ConcatAst.java | Refactors CONCAT to unlimited-args base and adds getName(). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/CoalesceAst.java | Refactors COALESCE to unlimited-args base and adds getName(). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/CeilAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/BoundAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/BooleanNotAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/BnodeAst.java | Adds getName() and visitor traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/BinaryRegexAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/AndAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/AddAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/AbstractUnlimitedArgumentsFunctionAst.java | New shared base implementing visitor traversal for unlimited-args functions. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/AbstractUnaryConstraintAst.java | Adds single-arg ctor, renames accessor, and adds visitor traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/AbstractBinaryFunctionAst.java | Makes binary function base abstract (prevents direct instantiation). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/AbstractBinaryConstraintAst.java | Adds visitor traversal for binary constraints. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/AbsAst.java | Adds getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/ClearRequestAst.java | Adds visitor traversal for CLEAR update operation. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/BindAst.java | Adds visitor traversal for BIND patterns. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/BgpAst.java | Adds visitor traversal for BGP patterns. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/AskQueryAst.java | Adds visitor traversal for ASK query AST. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/AggregateAst.java | Adds visitor traversal and getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/validator/SparqlQuerySemanticValidator.java | Registers new semantic validation rules into the default validator pipeline. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/support/VisitableAst.java | New shared interface to unify accept(AstVisitor) across AST nodes. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/support/VariableScopeAnalyzer.java | Updates traversal for renamed unary accessor and unlimited-args functions. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/support/SemanticValidationUtils.java | New helper utilities for “potential boolean/numeric/IRI/unknown” type checks. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/support/AstVisitor.java | New visitor API for semantic traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/support/AbstractAstVisitor.java | Default no-op visitor implementation for selective overrides. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/rule/OperandArgumentNumericTypeValidationRule.java | New rule to enforce numeric-only operands/functions. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/rule/OperandArgumentIRITypeValidationRule.java | New rule to disallow IRI ordering comparisons. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/rule/OperandArgumentBooleanTypeValidationRule.java | New rule to enforce boolean-only operands. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/rule/FilterArgumentsValidationRule.java | New rule to enforce FILTER/HAVING boolean-resolvable expressions. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/rule/AbstractSemanticValidationRule.java | Adds shared diagnostic constructors for scope and type errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds checks on functions that will only accept numerics an booleans. Other types can be assumed to be string if reduced to their lexical values.
The argument count is already check during the parsing step.