feat(validation): warn about missing variables used as bare condition operands - #231
Merged
Merged
Conversation
|
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.
Changes
ValidateTemplate(template, data)now warns when a condition tests a variable that is missing from the data for truthiness on its own (a bare operand). Such a variable is silently false, which usually hides a typo ({{#if IsActiv}}).ValidationWarningType.MissingConditionVariable = 2. The message names the variable and the condition:Condition 'A and Missing' uses variable 'Missing', which is not provided in the data. It is treated as false.Locationis the marker ({{#if A and Missing}}).ConditionalEvaluator.CollectBareOperandVariables): the whole condition, and the operands ofand/or/not. Nested operator nodes are always descended into.{{#if Missing}},{{#if not Missing}},{{#if A and Missing}},{{#elseif Missing}}, inline{{#if Missing}}x{{/if}}, table-row conditionals, headers/footers, and conditions inside loops. Inside loops the check uses the same scoped resolution as missing placeholders: item properties, named iteration variables, globals resolve, and@metadata/./thisare skipped. Conditions in loops over empty collections are not checked, the same as placeholders.exists/is empty/is not empty(they are meant for missing values); comparison operands (Status = Activebareword fallback stays as it is);inand the string operators; invalid conditions (already anInvalidConditionalExpressionerror); validation without data.{{(...)}}expressions are not checked (decision). Today the validator reports a whole inline expression as one missing variable (Variable '(A and B)' is referenced ... but not provided), even when every operand is present. That is a separate existing false positive. A bare-operand warning on top of it would report the same expression twice. The right fix is to validate inline expressions through the AST instead, which is a behavior change toErrors/IsValid, so it belongs in its own PR.MissingVariablesorErrors, andIsValiddoes not change. Existing DOCX consumers see only an additional warning.Core/MissingConditionVariableCheck.cs), called fromScopedVariableValidator(DOCX) and from theOdtTemplateValidatorscope walker (ODT). TheTemplateProcessorfacade therefore gets it too. Each condition/variable pair is reported once.Tests
Integration/ValidationMissingConditionVariableTests: every case runs as DOCX and ODT, both directly and through the facade, and asserts identical warnings/errors/missing variables. Covered: bare/not/and/or/grouping/elseif/inline/dotted path; no warning for exists / is empty / is not empty / comparisons / in / string operators / literals; no data; invalid condition; dedupe; several variables; loop scoping (item properties, named vars, @first/@last/@index, this, globals); missing item property in a loop; item property used outside its loop; empty loop; inline expressions; table-row conditional and header.Conditionals/CollectBareOperandVariablesTests: the AST walk.Odt/OdtValidationParityTests: new "missing bare condition operands" case.-p:ContinuousIntegrationBuild=true(0 warnings), all test projects on net10/9/8,dotnet format --verify-no-changes,dotnet pack,mkdocs build --strict.Public API impact
Additive:
ValidationWarningType.MissingConditionVariable = 2(inPublicAPI.Unshipped.txt, with XML docs). Behavior change limited toValidationResult.Warnings: validation with data can return additional warnings.Errors,MissingVariablesandIsValidare unchanged.