410 [SPARQL] Reject non-boolean expressions in FILTER and HAVING#455
Open
MaillPierre wants to merge 8 commits into
Open
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR extends the “next” SPARQL parser/AST with a visitor-based traversal API to support semantic validation, and introduces a new validation rule intended to reject non-boolean expressions used in FILTER (and per title/description, HAVING). It also updates a broad set of AST nodes and tests to align with the new visitor API and with renamed unary-argument accessors.
Changes:
- Introduces
VisitableAst/AstVisitor(+ defaultAbstractAstVisitor) and implementsaccept()traversal across the next-query AST hierarchy. - Adds and wires
FilterArgumentsValidationRuleintoSparqlQuerySemanticValidator, plus new/updated tests around invalidFILTER(...)forms. - Normalizes unary-argument accessors (
getArgument()→argument()) and updates bridge + parser tests accordingly; renames legacyASTVisitable→AstVisitable.
Reviewed changes
Copilot reviewed 133 out of 133 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/fr/inria/corese/core/next/query/impl/parser/SparqlParserValidationTest.java | Adds semantic validation tests for rejecting non-boolean FILTER expressions. |
| 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 FILTER tests to use boolean comparisons and updates unary accessors. |
| 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 accessors and adjusts numeric/unary FILTER cases to be boolean. |
| 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()). |
| src/test/java/fr/inria/corese/core/next/query/impl/parser/SparqlParserCoalesceTest.java | Adjusts COALESCE FILTER tests to be boolean comparisons; updates assertions accordingly. |
| 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 helper doc comment for aggregate extraction in tests. |
| src/main/java/fr/inria/corese/core/sparql/triple/parser/TopExp.java | Renames legacy visitable interface usage to AstVisitable. |
| src/main/java/fr/inria/corese/core/sparql/triple/parser/ASTQuery.java | Renames legacy visitable interface usage to AstVisitable. |
| src/main/java/fr/inria/corese/core/sparql/triple/api/AstVisitable.java | Renames legacy interface ASTVisitable → AstVisitable. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/SparqlAstToExpression.java | Updates unary-argument accessors used when converting AST to Corese expressions. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/VarAst.java | Adds visitor support and getName() implementation for variables. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/ValuesAst.java | Makes VALUES visitable and adds traversal over mappings. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/ValueMappingAst.java | Makes value mappings visitable and adds traversal over map entries. |
| 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 adds subject/predicate/object traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/TermAst.java | Extends VisitableAst and introduces required getName() for diagnostics. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/SubQueryAst.java | Adds visitor traversal for subqueries. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/SolutionModifierAst.java | Makes modifiers visitable and adds traversal over GROUP BY / ORDER BY / HAVING. |
| 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 queries across major components. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/QueryPrologueAst.java | Makes prologue visitable and traverses prefixes/base. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/QueryAst.java | Makes queries visitable (extends VisitableAst). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/ProjectionAst.java | Makes projection visitable and traverses projected variables. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/PrefixDeclarationAst.java | Makes prefix declarations visitable and traverses namespace IRI. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/PatternAst.java | Makes patterns visitable (extends VisitableAst) and standardizes accept. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/OrderConditionAst.java | Makes ORDER BY conditions visitable and traverses expression. |
| 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/LiteralAst.java | Adds visitor support and getName() implementation for literals. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/IriAst.java | Adds visitor support and getName() implementation for IRIs. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/HavingAst.java | Makes HAVING visitable and traverses conditions. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/GroupGraphPatternAst.java | Adds visitor traversal over group graph patterns. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/GroupByAst.java | Makes GROUP BY visitable and traverses expressions. |
| 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 queries. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/DatasetClauseAst.java | Makes dataset clause visitable and traverses graph IRIs. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/ConstructTemplateAst.java | Makes construct templates visitable and traverses template triples. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/ConstructQueryAst.java | Adds visitor traversal for CONSTRUCT queries. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/ConstraintAst.java | Extends visitability for constraint nodes. |
| 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 support. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/UnlimitedArgumentsFunctionAst.java | Introduces marker interface for variadic functions. |
| 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() (currently used 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 support. |
| 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 to unary base 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 support. |
| 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 support. |
| 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 for IRI-based function calls. |
| 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 into variadic base class and adds getName(). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/CoalesceAst.java | Refactors COALESCE into variadic base class 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 | Adds shared variadic function base with visitor traversal. |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/AbstractUnaryConstraintAst.java | Adds visitor traversal and renames unary accessor to argument(). |
| src/main/java/fr/inria/corese/core/next/query/impl/sparql/ast/constraint/AbstractBinaryFunctionAst.java | Makes binary-function base abstract. |
| 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/parser/semantic/validator/SparqlQuerySemanticValidator.java | Wires new filter validation rule into semantic validator pipeline. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/support/VisitableAst.java | Introduces base interface for visitor-enabled AST nodes. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/support/VariableScopeAnalyzer.java | Updates unary/variadic AST handling for variable scope analysis. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/support/AstVisitor.java | Introduces AST visitor interface for semantic traversals. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/support/AbstractAstVisitor.java | Provides default no-op visitor implementation. |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/rule/FilterArgumentsValidationRule.java | Adds rule to validate FILTER argument types (and intended HAVING). |
| src/main/java/fr/inria/corese/core/next/query/impl/parser/semantic/rule/AbstractSemanticValidationRule.java | Adds reusable incorrect-type diagnostic builder. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
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>
6890f72 to
124ec86
Compare
|
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 a visitor design pattern to ASTs to facilitate validation of rules that ned to explore recursively the patterns.
Check that the arguments of a a filter are either a boolean expression or a literal expresion that is not datetime related or numeric, or a function call, or a variable or a literal.