feat: add support wild card object props #56
Conversation
Also support wild card selector mappings
WalkthroughThe recent updates primarily introduce support for handling wildcard property values in JSON templates. This involves changes across various files to accommodate new methods and logic for processing wildcard selectors. Key modifications include updates to the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant JsonTemplateEngine
participant JsonTemplateLexer
participant JsonTemplateParser
participant JsonTemplateTranslator
User->>JsonTemplateEngine: convertMappingsToTemplate(mappings, options)
JsonTemplateEngine->>JsonTemplateLexer: matchObjectWildCardPropValue()
JsonTemplateEngine->>JsonTemplateParser: parseObjectPropWildcardValueExpr()
JsonTemplateParser->>JsonTemplateTranslator: translateObjectWildcardValueExpr()
JsonTemplateTranslator-->>JsonTemplateEngine: Translated Template
JsonTemplateEngine-->>User: Return Translated Template
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Coverage report
Test suite run success162 tests passing in 3 suites. Report generated by 🧪jest coverage report action from 001011d |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #56 +/- ##
===========================================
Coverage 100.00% 100.00%
===========================================
Files 10 14 +4
Lines 3078 4546 +1468
Branches 726 1074 +348
===========================================
+ Hits 3078 4546 +1468 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (2)
src/translator.ts (1)
Line range hint
899-899: Correct the use ofthisin a static context.- return `${varName} = ${varName}.length < 2 ? ${varName}[0] : ${varName};`; + return `${JsonTemplateTranslator.name}.returnIsNotEmpty(${varName}) ? (${varName} = Array.isArray(${varName}) ? ${varName} : [${varName}]) : ${varName};`;The static method
convertToSingleValueIfSafeincorrectly usesthiswhich can lead to confusion and potential errors in a static context. Replacethiswith the class nameJsonTemplateTranslatorto correctly reference the static method.src/parser.ts (1)
Line range hint
1477-1477: Refactor to avoid usingthisin static methods.Static methods should not use
thisas it can lead to confusion and potential runtime errors. Replacethiswith the class nameJsonTemplateParser.- this.pathContainsVariables(newPathExpr.parts); + JsonTemplateParser.pathContainsVariables(newPathExpr.parts); - this.isRichPath(newPathExpr); + JsonTemplateParser.isRichPath(newPathExpr); - this.convertToFunctionCallExpr(fnExpr, newPathExpr); + JsonTemplateParser.convertToFunctionCallExpr(fnExpr, newPathExpr); - this.convertToBlockExpr(expr); + JsonTemplateParser.convertToBlockExpr(expr);Also applies to: 1483-1483, 1490-1490, 1521-1521
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (14)
- src/engine.ts (1 hunks)
- src/lexer.ts (1 hunks)
- src/parser.ts (6 hunks)
- src/reverse_translator.ts (3 hunks)
- src/translator.ts (3 hunks)
- src/types.ts (2 hunks)
- src/utils/converter.ts (7 hunks)
- test/scenarios/mappings/data.ts (4 hunks)
- test/scenarios/mappings/invalid_object_mappings.json (1 hunks)
- test/scenarios/mappings/object_mappings.json (1 hunks)
- test/scenarios/objects/data.ts (1 hunks)
- test/scenarios/objects/invalid_wild_cards.jt (1 hunks)
- test/scenarios/objects/wild_cards.jt (1 hunks)
- test/utils/scenario.ts (1 hunks)
Files skipped from review due to trivial changes (4)
- test/scenarios/mappings/invalid_object_mappings.json
- test/scenarios/mappings/object_mappings.json
- test/scenarios/objects/invalid_wild_cards.jt
- test/scenarios/objects/wild_cards.jt
Additional context used
Biome
test/utils/scenario.ts
[error] 6-36: Avoid classes that contain only static members. (lint/complexity/noStaticOnlyClass)
Prefer using simple functions instead of classes with only static members.
src/engine.ts
[error] 22-22: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 29-29: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 51-51: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 58-58: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 70-70: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 74-74: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 74-74: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 83-83: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 83-83: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 11-11: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
[error] 13-13: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
[error] 20-20: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
[error] 25-25: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
src/lexer.ts
[error] 366-366: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.src/translator.ts
[error] 899-899: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.src/parser.ts
[error] 1477-1477: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 1483-1483: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 1490-1490: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 1521-1521: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
Additional comments not posted (24)
test/scenarios/objects/data.ts (2)
4-7: LGTM! The test scenario for invalid wildcards is well-defined.
15-35: LGTM! The test scenario for valid wildcards is comprehensive and well-structured.test/scenarios/mappings/data.ts (3)
160-160: LGTM! The test scenario for invalid array mappings is well-defined.
166-173: LGTM! The test scenario for invalid object mappings is comprehensive and well-structured.
254-288: LGTM! The test scenario for object mappings with wildcards is comprehensive and well-structured.src/types.ts (2)
87-87: LGTM! The addition ofOBJECT_PROP_WILD_CARD_VALUE_EXPRis necessary for supporting wildcard properties.
151-153: LGTM! TheObjectWildcardValueExpressioninterface is well-defined and necessary for handling wildcard expressions.src/utils/converter.ts (5)
86-88: LGTM! TheisWildcardSelectorfunction correctly identifies wildcard selectors.
90-95: LGTM! ThecreateWildcardObjectPropValueExpressionfunction is correctly implemented for creating wildcard property expressions.
97-133: LGTM! TheprocessWildCardSelectorfunction is comprehensive and correctly handles wildcard selectors in mappings.
136-156: LGTM! ThehandleNextPartfunction effectively manages the processing of the next part in flat mappings.
201-201: LGTM! TheconvertToObjectMappingfunction is well-implemented and crucial for converting flat mappings to object expressions.src/reverse_translator.ts (2)
123-125: The implementation oftranslateWildcardObjectPropValueExpressioncorrectly translates wildcard object property expressions.
114-117: The addition of theOBJECT_PROP_WILD_CARD_VALUE_EXPRcase intranslateExpressionmethod correctly handles the new expression type.src/lexer.ts (1)
96-98: The implementation ofmatchObjectWildCardPropValuecorrectly identifies wildcard property values.src/translator.ts (4)
193-198: Add handling forOBJECT_PROP_WILD_CARD_VALUE_EXPRintranslateExpr.This addition correctly integrates the new wildcard property expression type into the translation process, ensuring that expressions of this type are handled appropriately.
535-541: Implement translation forObjectWildcardValueExpression.The method
translateObjectWildcardValueExprcorrectly assigns the wildcard expression value to the destination variable. This is a straightforward and effective implementation for handling wildcard values in object expressions.
543-559: Implement translation for wildcard properties in objects.The method
translateObjectWildcardPropeffectively handles the translation of object properties that are marked as wildcards. The use ofObject.entriesto iterate over properties and the dynamic assignment based on the wildcard key expression are correctly implemented.
567-590: EnhancetranslateObjectExprto support wildcard properties.This modification to
translateObjectExpradds support for translating object expressions that contain wildcard properties. The conditional handling based on thewildcardproperty of each object property expression is well-integrated.src/parser.ts (5)
28-28: AddedObjectWildcardValueExpressionto imports.This addition is necessary for the new functionality related to wildcard object properties.
1088-1098: New methodparseObjectPropWildcardValueExprcorrectly throws an error for invalid wildcard property values.This method enhances error handling by providing clear feedback on incorrect wildcard usage, which is crucial for debugging and user experience.
1110-1111: Integration ofparseObjectPropWildcardValueExprinparseObjectKeyExpr.This integration allows the parser to handle wildcard properties directly within object keys, aligning with the PR's objectives to support wildcard properties.
Line range hint
1142-1154: MethodisWildcardPropKeychecks if an expression is a wildcard property key.This utility method is essential for determining when to treat properties as wildcards, which is a key part of supporting flexible JSON structures.
1390-1392: Handling of wildcard property values inparsePrimaryExpr.Properly extends the parser's capabilities to recognize and process wildcard property values at a fundamental level.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- test/scenarios/objects/data.ts (1 hunks)
- test/scenarios/objects/wild_cards.jt (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- test/scenarios/objects/data.ts
- test/scenarios/objects/wild_cards.jt
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
test/e2e.test.ts (1)
Line range hint
19-36: Consider replacingforEachwithfor...offor better performance.- scenarios.forEach((scenarioFileName) => { + for (const scenarioFileName of scenarios) {src/engine.ts (1)
Line range hint
11-11: ReplaceFunctiontype with more specific function signatures to prevent potential bugs.- private readonly fn: Function; + private readonly fn: (data: unknown, bindings: Record<string, unknown>) => unknown;Also applies to: 13-13, 20-20, 29-29
Tools
Biome
[error] 20-20: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
[error] 29-29: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/engine.ts (4 hunks)
- test/e2e.test.ts (1 hunks)
- test/scenario.test.ts (1 hunks)
- test/utils/scenario.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- test/scenario.test.ts
Files skipped from review as they are similar to previous changes (1)
- test/utils/scenario.ts
Additional context used
Biome
test/e2e.test.ts
[error] 19-36: Prefer for...of instead of forEach. (lint/complexity/noForEach)
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
src/engine.ts
[error] 11-11: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
[error] 13-13: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
[error] 20-20: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
[error] 29-29: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
Additional comments not posted (1)
test/e2e.test.ts (1)
4-4: LGTM! The change to a wildcard import aligns with the PR's objective to handle wildcard properties.
|
What are the changes introduced in this PR?
Add support for wild card object props, also support wild card selector mappings
What is the related Linear task?
Resolves INT-2160
Please explain the objectives of your changes below
Put down any required details on the broader aspect of your changes. If there are any dependent changes, mandatorily mention them here
Any changes to existing capabilities/behaviour, mention the reason & what are the changes ?
N/A
Any new dependencies introduced with this change?
N/A
Any new generic utility introduced or modified. Please explain the changes.
N/A
Any technical or performance related pointers to consider with the change?
N/A
@coderabbitai review
Developer checklist
My code follows the style guidelines of this project
No breaking changes are being introduced.
All related docs linked with the PR?
All changes manually tested?
Any documentation changes needed with this change?
Is the PR limited to 10 file changes?
Is the PR limited to one linear task?
Are relevant unit and component test-cases added?
Reviewer checklist
Is the type of change in the PR title appropriate as per the changes?
Verified that there are no credentials or confidential data exposed with the changes.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests