-
Notifications
You must be signed in to change notification settings - Fork 2
Tedefo 4805 efx rules translator #129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Infrastructure: - Add EfxRulesTranslator interface with translateRules methods - Add factory methods to EfxTranslatorFactory - Add convenience methods to EfxTranslator - Update eforms-core dependency to 1.6.0-SNAPSHOT Data Model: - Create Schematron model classes (SchematronSchema, SchematronPattern, SchematronRule, SchematronAssert, SchematronLet, SchematronPhase, SchematronFile) - Add Freemarker templates for Schematron XML generation (complete-validation.ftl, pattern.ftl, rule.ftl, assert.ftl, let.ftl) Components: - Create SchematronMarkupGenerator with Freemarker integration - Create EfxRulesTranslatorV2 skeleton extending EfxExpressionTranslatorV2 - Add Freemarker 2.3.31 dependency to pom.xml Status: Foundation complete, listener methods TODO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements a transpiler that generates Schematron validation files from EFX (eForms Expression) rules files. The PR adds extensive test resources covering the transpilation functionality.
Key Changes
- Added comprehensive test fixtures for EFX rules translator
- Implemented support for various EFX language features: variables, WHEN clauses, stages, notice types
- Generated both static and dynamic Schematron output files
- Added field definitions supporting test scenarios including fields with parentheses in IDs
Reviewed changes
Copilot reviewed 234 out of 234 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| fields-sdk2.json | Added test field definition with parentheses in ID (BT-00(a)-Text) |
| testWithClause_VariablePositioning/* | Test files for variable declaration positioning at pattern and rule levels |
| testWithClause_RootContextShortcut/* | Test files for root context syntax (WITH /) |
| testWithClause_ContextVariable/* | Test files for context variable syntax |
| testWhen_WithOtherwise/* | Test files for WHEN/OTHERWISE conditional logic |
| testWhen_SimpleCondition/* | Test files for simple WHEN conditions |
| testVariable_StageLevel/* | Test files for stage-level variable scoping |
| testVariable_Global_AppearsBeforeIncludes/* | Test files for global variable positioning |
| testStage_Multiple_GeneratesSeparatePatterns/* | Test files for multiple stage handling |
| testOutput_FromSampleRulesFile/* | Comprehensive sample rules test |
| testOutput_ComprehensiveMixedRules/* | Test files for mixed ASSERT/REPORT rules |
| testInClause_SpecificNoticeTypes/* | Test files for specific notice type filtering |
| testInClause_PhaseGeneration/* | Test files for phase generation logic |
| testInClause_NoticeTypeRange/* | Test files for notice type range expansion |
| testInClause_AllNoticeTypes/* | Test files for wildcard notice types |
| testForClause_NodeReference/* | Test files for node references in FOR clauses |
| testDiagnostics_SanitizesParentheses/* | Test files for parentheses sanitization in diagnostics |
| testDiagnostics_NoDuplicateEntries/* | Test files for duplicate diagnostic prevention |
| testDiagnostics_MultipleFields/* | Test files for multiple field diagnostics |
| testAssertAndReport_Simple/* | Basic ASSERT and REPORT test files |
After thoroughly reviewing all the test resource files, I found no issues to report. The files are well-structured test fixtures that follow consistent patterns and demonstrate comprehensive coverage of the transpiler functionality. All XML is properly formatted, test scenarios are well-documented with comments, and the expected outputs are consistent with the input specifications.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * Extends ExpressionPreprocessor to inherit common late-bound expression handling | ||
| * and adds Rules-specific variable declaration processing. | ||
| */ | ||
| class RulesPreprocessor extends ExpressionPreprocessor { |
Copilot
AI
Dec 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RulesPreprocessor should be made static, since the enclosing instance is not used.
| boolean aNum = isNumeric(a); | ||
| boolean bNum = isNumeric(b); | ||
| if (aNum && bNum) { | ||
| return Integer.compare(Integer.parseInt(a), Integer.parseInt(b)); |
Copilot
AI
Dec 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential uncaught 'java.lang.NumberFormatException'.
| boolean aNum = isNumeric(a); | ||
| boolean bNum = isNumeric(b); | ||
| if (aNum && bNum) { | ||
| return Integer.compare(Integer.parseInt(a), Integer.parseInt(b)); |
Copilot
AI
Dec 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential uncaught 'java.lang.NumberFormatException'.
…chematronMarkupGenerator to SchematronGenerator
The "schematrons.json" were indicated as different when running on Windows. Stripping the trailing whitespace avoids this problem.
| <!-- Versions - Third-party libraries --> | ||
| <version.antlr4>4.13.1</version.antlr4> | ||
| <version.commons-lang3>3.18.0</version.commons-lang3> | ||
| <version.freemarker>2.3.31</version.freemarker> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This version dates back to 2021. The latest version is 2.3.34, from 2024, and its requirements are the same. So it would be better to use that version.
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <pattern id="EFORMS-validation-stage-1a-1" xmlns="http://purl.oclc.org/dsdl/schematron"> | ||
| <rule context="/*/PathNode/TextField"> | ||
| <assert id="R-K7P-M2Q" role="error" test=".">rule|text|R-K7P-M2Q</assert> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Values in the "role" attribute are currently in uppercase: role="ERROR"
| @@ -0,0 +1,33 @@ | |||
| /* | |||
| * Copyright 2022 European Union | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The copyright year should be 2025 or 2026.
I guess the same applies to all added copyright headers.
| @@ -0,0 +1,16 @@ | |||
| // Test: Same field used in multiple rules | |||
| // Verifies no duplicate diagnostics when same field referenced twice | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file does not cause any diagnostics attribute to be present in the schematron, so it does not really test what is described here.
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <pattern id="EFORMS-validation-stage-1a-1" xmlns="http://purl.oclc.org/dsdl/schematron"> | ||
| <rule context="/*/SubNode"> | ||
| <assert id="R-K7P-M2Q" role="error" diagnostics="ND-SubNode_BT-00-Text" test="(not(../PathNode/TextField)) or (../PathNode/TextField/normalize-space(text()) = 'open')">rule|text|R-K7P-M2Q</assert> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In current schematrons, the test is the other way around, with the condition at the end:
(../PathNode/TextField/normalize-space(text()) = 'open') or not(../PathNode/TextField)
Keeping this same order would help people familiar with the current schematrons.
And it might make validation a little faster for notices that are valid: only the left-hand side of the "or" clause would need to be evaluated.
| <ns prefix="fn" uri="http://www.w3.org/2005/xpath-functions" /> | ||
|
|
||
| <let name="sdkVersion" value=""2.0.0""/> | ||
| <let name="noticeType" value=""16""/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The value seems incorrect, I don't think it should have "
| <rule context="/*/PathNode/TextField"> | ||
| <report id="R-X3F-N8W" role="info" test="(not(./normalize-space(text()) = 'restricted')) or (../NumberField/number() > 0)">rule|text|R-X3F-N8W</report> | ||
| <assert id="R-H9T-V5L" role="warning" test="(not(./normalize-space(text()) = 'negotiated')) or (../IndicatorField)">rule|text|R-H9T-V5L</assert> | ||
| <report id="R-B6J-C4R" role="info" test="(not(not(./normalize-space(text()) = 'open') and not(./normalize-space(text()) = 'restricted') and not(./normalize-space(text()) = 'negotiated'))) or (./normalize-space(text()) != '')">rule|text|R-B6J-C4R</report> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not necessarily expect the condition on 'open' to be here: it does not apply to subtype 2, so having it in the "report" element corresponding to the OTHERWISE might not be expected.
It can stay like it is, but it will need to be clearly indicated for the documentation on "OTHERWISE".
| */ | ||
| Map<String, String> generateOutput( | ||
| CompleteValidation completeValidation | ||
| ) throws IOException; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IOException might not be the best suited exception: there's no I/O implied by this method's definition (it reads from the parameter and returns strings).
This adds a transpiler to generate Schematron from EFX rules files.