feat(query): port IsError/ErrorReason from kuribara-hideaki fork#363
Open
berezovskyi wants to merge 13 commits into
Open
feat(query): port IsError/ErrorReason from kuribara-hideaki fork#363berezovskyi wants to merge 13 commits into
berezovskyi wants to merge 13 commits into
Conversation
This concludes my attempt to analyze the `kuribara-hideaki/oslc4net` fork and integrate its changes.
Due to significant divergence and some limitations I encountered that prevented complex manual code merging, I wasn't able to complete the full integration of the fork's code enhancements.
However, as you requested, I have included the following:
1. An updated `.gitignore` file based on the fork's improvements.
2. Conceptual (placeholder) integration of some smaller fixes.
3. A new suite of unit tests added to `OSLC4Net.Core.QueryTests.QueryBasicTest.cs`. These tests are designed to verify the functionality of key enhancements identified in the fork, such as:
- Improved error handling in query parsing (`IsError`/`ErrorReason` properties).
- Correct parsing of `*` as an operand in queries.
- Changes to `SortTerms` structure.
These newly added tests are expected to FAIL, as the underlying code logic from the fork has not been fully implemented in this branch. They serve as a specification and starting point for you to manually port the desired features from the fork.
The `integrate-fork-kuribara` branch contains all changes I made during this process.
This comment was marked as outdated.
This comment was marked as outdated.
Member
Author
|
also relevant: eclipse-lyo/lyo#43 |
# Conflicts: # OSLC4Net_SDK/Tests/OSLC4Net.Core.QueryTests/QueryBasicTest.cs
Ports the error-as-property pattern from kuribara-hideaki/oslc4net so that ParseWhere and ParseOrderBy report parse failures via IsError / ErrorReason on the returned clause instead of throwing ParseException. This matches the behaviour the failing-test commit (347e5ab) was written against and makes the 5 placeholder tests pass without regenerating the ANTLR parsers. Source commits in the fork: db49995 — IsError/ErrorReason on clauses (the IBaseClause interface) efcacd7 — orderBy parse-error tolerance 127e068 — where 'in' parse-error tolerance Changes: - New IBaseClause interface with IsError + ErrorReason. - WhereClause and SortTerms (OrderByClause's base) extend it. - WhereClauseImpl and SortTermsImpl gain an error-only constructor and implement IsError/ErrorReason. - QueryUtils.ParseWhere / ParseOrderBy now catch RecognitionException and recursively scan the parse tree for CommonErrorNodes, returning an error-state impl in either case (no more throws on invalid input). - Existing BasicWhereTest / BasicOrderByTest parametrized cases now assert against IsError instead of catching ParseException. - The two placeholder tests with grammar-incompatible expressions (whitespace around `=`, unprefixed orderBy term) use grammar-valid expressions that preserve the test intent (quoted asterisk operand, Children property access). Not ported: SortTerms.Children -> IList<ITree> (fork regression that threw away SimpleSortTermImpl/ScopedSortTermImpl resolution); bare asterisk as where-clause value (requires grammar regeneration). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Documentation-only follow-up to the IsError port (commit d14d4ae): - CHANGELOG.md gets two entries: an Added line for IBaseClause / IsError / ErrorReason, and a Changed (breaking) line noting that ParseWhere / ParseOrderBy no longer throw on invalid input. - OslcWhere.g gains a conformance-notes block citing the OSLC Query v3.0 spec (open-projects), calling out the wildcard rule (allowed in property names only, not as a value), the bake-in whitespace around `and`/`in`, and the dead ANTLR3 CSharp3 toolchain. - GeneratingParsers.txt rewritten to capture the current state: the CSharp3 target crashes StringTemplate on every Java/ST4 combo we tested (zulu 8 through temurin 21), so .g edits do not propagate without an ANTLR 4 migration or hand-patching. eclipse-lyo's identical grammar regenerates fine because they target Java. No code changes; bare-asterisk-as-value (kuribara-hideaki fork) is documented as not-ported (it's a non-spec extension and eclipse-lyo doesn't ship it either). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
berezovskyi
changed the title
2 tasks
Per the policy in AGENTS.md (added in #754), every file touched substantively in this PR gets the standard attribution line. - IBaseClause.cs (new): sole attribution; the IBM 2013 header that was copy-pasted by mistake is replaced. - QueryUtils.cs, WhereClauseImpl.cs, SortTermsImpl.cs, QueryBasicTest.cs, Grammars/OslcWhere.g (modified): line appended after the existing IBM Corporation header; all other attributions and the Contributors block preserved. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Member
Author
|
@kuribara-hideaki @nenaaki @m-yoko I noticed that you used OSLC4Net in the past and made improvements around the robustness of the OSLC Query parsing. May I ask you for help to review this PR? Are you still using the library? If so, could you please describe what is missing for you before you can switch away from your fork (I am assuming here) back to the mainline repo and our NuGet packages? Thank you in advance! |
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
1 participant
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.
Summary
Integrate the OSLC Query enhancements from kuribara-hideaki/oslc4net.
What's ported
db49995—IsError/ErrorReasonas properties on clausesefcacd76— orderBy parse-error tolerance127e068a—where … in […(unclosed list) toleranceCode changes:
IBaseClauseinterface withIsError(bool) andErrorReason(string?).WhereClauseandSortTerms(and thereforeOrderByClause) now extend it.WhereClauseImplandSortTermsImplgain error-state constructors.QueryUtils.ParseWhere/ParseOrderBycatchRecognitionExceptionand recursively walk the parse tree forCommonErrorNodes, returning an error-state clause instead of throwing. The recursive walk is what catches deep failures likeqm:state in ["Open"orqm:testcase=.❗️ Breaking change
QueryUtils.ParseWhereandQueryUtils.ParseOrderByno longer throwParseExceptionon invalid input. Callers must inspectclause.IsError(and optionallyErrorReason) instead. Audit performed (see comment); only one prod method per parser is affected; only the in-tree parameterized tests needed updates. The other 4 parsers (ParsePrefixes,ParseSelect,parseProperties,ParseSearchTerms) still throw — out of scope for this PR.What's NOT ported (and why)
834cddd—SortTerms.Children→IList<ITree>SimpleSortTermImpl/ScopedSortTermImplresolution; ours is more complete.31d2858— bare*as where-clause value (e.g.qm:title=*)qm:title="*"instead — that works today. SeeOslcWhere.gheader comment andImpl/GeneratingParsers.txtfor the regen blocker write-up.