fix(validation): validate inline expressions via the condition AST instead of reporting them as one missing variable - #233
Merged
Conversation
…stead of reporting them as one missing variable
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
Problem
ValidateTemplate(WordTemplateValidator/ScopedVariableValidator, OpenDocumentOdtTemplateValidator, and theTemplateProcessorfacade) treated an inline expression placeholder such as{{(A and B)}},{{(IsActive):yesno}}or{{(Price > 100)}}as a single variable named(A and B):Variable '(A and B)' is referenced in the template but not provided in the data.as aMissingVariableerror, and listed(A and B)inMissingVariables, even when every operand was present. SoIsValidwasfalsefor a correct template.AllPlaceholderscontained the expression text ((A and B)) and not the variables it uses.{{(A and)}},{{(A && B)}}) were not reported. Processing leaves them unreplaced with anExpressionFailedwarning.Fix
The new
Core/InlineExpressionValidation.csis shared by both validators. It parses inline expressions with the same parser processing uses (ConditionAstCache.InlineExpressions, which also allows single-quoted strings):InvalidConditionalExpressionerror (Invalid expression '(A and)': ..., location = the placeholder). It is reported with or without data, once per message.AllPlaceholders: gets the variables the expression references, taken from the AST (as for{{#if}}conditions). Operators, literals and keywords are not listed. An expression that cannot be parsed adds nothing. Word and OpenDocument behave the same.MissingVariableerror (Variable 'X' is referenced in expression '(...)' but not provided in the data.) and aMissingVariablesentry are reported for each variable that processing reads from the data without a fallback and that cannot be resolved in the loop scope. Item properties, named iteration variables,@metadata and./thisresolve as in processing. The checked variables are:and/or/not. A missing one evaluates to false.in,contains,startswithandendswith. These are resolved strictly, so a missing one is null.ResolveOrLiteral, so an unresolved bareword is a string literal ({{(Status = Active)}},{{(Missing > 100)}}).exists,is emptyandis not empty.MissingConditionVariablewarning (feat(validation): warn about missing variables used as bare condition operands #231): not added for inline expressions. Their missing truthiness operands are alreadyMissingVariableerrors, so the warning would repeat them.Tests
Integration/ValidationInlineExpressionTests.cs. Every case is validated as Word and as OpenDocument, directly and through the facade, and the results must be identical. Cases:in/contains/startswith,exists/is empty, literals). For each one the test also processes the template and asserts no missing variables and no warnings, so validation predicts processing.AllPlaceholderscontent.in/string operators,(Missing) = truenot reported).ExpressionFailedwarning and that the text is left unchanged.OdtValidationParityTests: new "inline expressions" template.ValidationMissingConditionVariableTests: the inline test now also asserts theMissingVariableerror.dotnet format --verify-no-changes,dotnet packandmkdocs build --strictpass.Docs:
docs/for-developers/quick-start.md,TriasDev.Templify/README.md(ValidationResult),ARCHITECTURE.md.Public API impact: none (validation results: false errors removed)
This is a bug fix to validation results of the shipped
ValidateTemplateAPI. There are no signature changes.MissingVariableerrors andMissingVariablesentries for inline expressions whose operands are present are gone.Binstead of(A and B)).AllPlaceholderslists an expression's variables instead of the expression text.MissingVariablefor genuinely missing operands, andInvalidConditionalExpressionfor expressions that processing cannot evaluate (also without data).