Skip to content

fix(markdown-template): preserve parent scope in conditional and optional blocks #647

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.

Sign up for GitHub

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

Open
yashhzd wants to merge 2 commits into
base: main
Choose a base branch
from
+168 −19
Open

fix(markdown-template): preserve parent scope in conditional and optional blocks #647

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb25333
fix(markdown-template): preserve parent scope in conditional and opti…
yashhzd Feb 26, 2026
eb6904f
fix(markdown-template): address review feedback for scope handling
yashhzd Mar 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions packages/markdown-template/lib/TypeVisitor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,11 @@ class TypeVisitor {
_throwTemplateExceptionForElement('Unknown property: ' + thing.name, thing);
}
if (thing.name === 'this') {
var property = currentModel; // BUG... if we are iterating over an array
var property = parameters.primitiveProperty || currentModel; // BUG... if we are iterating over an array
// of complex types using a {{this}}, then thing will be a ClassDeclaration or an
// EnumDeclaration!!

if (property && property.getType) {
if (property && property.getType && typeof property.isPrimitive === 'function') {
var _property$isRelations;
var serializer = parameters.templateMarkModelManager.getSerializer();
thing.decorators = processDecorators(serializer, property);
Expand Down Expand Up @@ -298,7 +298,6 @@ class TypeVisitor {
case 'ConditionalDefinition':
{
var _property5 = currentModel.getOwnProperty(thing.name);
var _nextModel2;
if (thing.name !== 'if' && !_property5) {
// hack, allow the node to have the name 'if'
_throwTemplateExceptionForElement('Unknown property: ' + thing.name, thing);
Expand All @@ -309,25 +308,26 @@ class TypeVisitor {
// }
var _serializer6 = parameters.templateMarkModelManager.getSerializer();
thing.decorators = _property5 ? processDecorators(_serializer6, _property5) : null;
_nextModel2 = _property5;
// Conditional blocks do not change scope — variables inside #if
// must resolve against the parent model, not the condition property
TypeVisitor.visitChildren(this, thing, {
templateMarkModelManager: parameters.templateMarkModelManager,
introspector: parameters.introspector,
model: _nextModel2,
model: currentModel,
kind: parameters.kind
}, 'whenTrue');
TypeVisitor.visitChildren(this, thing, {
templateMarkModelManager: parameters.templateMarkModelManager,
introspector: parameters.introspector,
model: null,
model: currentModel,
kind: parameters.kind
}, 'whenFalse');
}
break;
case 'OptionalDefinition':
{
var _property6 = currentModel.getOwnProperty(thing.name);
var _nextModel3;
var _nextModel2;
if (!_property6) {
_throwTemplateExceptionForElement('Unknown property: ' + thing.name, thing);
}
Expand All @@ -338,21 +338,26 @@ class TypeVisitor {
thing.decorators = processDecorators(_serializer7, _property6);
if (_property6.isPrimitive()) {
thing.elementType = _property6.getFullyQualifiedTypeName();
_nextModel3 = _property6;
// For primitive optional properties, keep the parent model as the
// current scope so that named variables (e.g. {{age}}) resolve
// against the parent class. The property is stashed separately
// so that {{this}} can still resolve to the primitive type.
_nextModel2 = currentModel;
} else {
thing.elementType = _property6.getFullyQualifiedTypeName();
_nextModel3 = parameters.introspector.getClassDeclaration(thing.elementType);
_nextModel2 = parameters.introspector.getClassDeclaration(thing.elementType);
}
TypeVisitor.visitChildren(this, thing, {
templateMarkModelManager: parameters.templateMarkModelManager,
introspector: parameters.introspector,
model: _nextModel3,
model: _nextModel2,
primitiveProperty: _property6.isPrimitive() ? _property6 : null,
kind: parameters.kind
}, 'whenSome');
TypeVisitor.visitChildren(this, thing, {
templateMarkModelManager: parameters.templateMarkModelManager,
introspector: parameters.introspector,
model: null,
model: currentModel,
kind: parameters.kind
}, 'whenNone');
}
Expand Down
21 changes: 13 additions & 8 deletions packages/markdown-template/src/TypeVisitor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,11 @@ class TypeVisitor {
_throwTemplateExceptionForElement('Unknown property: ' + thing.name, thing);
}
if (thing.name === 'this') {
const property = currentModel; // BUG... if we are iterating over an array
const property = parameters.primitiveProperty || currentModel; // BUG... if we are iterating over an array
// of complex types using a {{this}}, then thing will be a ClassDeclaration or an
// EnumDeclaration!!

if (property && property.getType) {
if (property && property.getType && typeof property.isPrimitive === 'function') {
const serializer = parameters.templateMarkModelManager.getSerializer();
thing.decorators = processDecorators(serializer,property);
if (property.isTypeEnum && property.isTypeEnum()) {
Expand Down Expand Up @@ -288,7 +288,6 @@ class TypeVisitor {
break;
case 'ConditionalDefinition': {
const property = currentModel.getOwnProperty(thing.name);
let nextModel;
if (thing.name !== 'if' && !property) { // hack, allow the node to have the name 'if'
_throwTemplateExceptionForElement('Unknown property: ' + thing.name, thing);
}
Expand All @@ -298,17 +297,18 @@ class TypeVisitor {
// }
const serializer = parameters.templateMarkModelManager.getSerializer();
thing.decorators = property ? processDecorators(serializer,property) : null;
nextModel = property;
// Conditional blocks do not change scope — variables inside #if
// must resolve against the parent model, not the condition property
TypeVisitor.visitChildren(this, thing, {
templateMarkModelManager:parameters.templateMarkModelManager,
introspector:parameters.introspector,
model:nextModel,
model:currentModel,
kind:parameters.kind
}, 'whenTrue');
Comment on lines 303 to 307
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ConditionalDefinition now correctly keeps model: currentModel for the whenTrue branch, but the {{else}} branch (whenFalse) is still visited with model: null later in this case. That makes any {{variable}} inside {{else}} fail type-checking, even though runtime conversion (ToCiceroMarkVisitor) evaluates both branches with the same parameters.data. Consider passing currentModel for whenFalse as well so variable resolution is consistent across both branches.

Copilot uses AI. Check for mistakes.
TypeVisitor.visitChildren(this, thing, {
templateMarkModelManager:parameters.templateMarkModelManager,
introspector:parameters.introspector,
model:null,
model:currentModel,
kind:parameters.kind
}, 'whenFalse');
}
Expand All @@ -326,7 +326,11 @@ class TypeVisitor {
thing.decorators = processDecorators(serializer,property);
if (property.isPrimitive()) {
thing.elementType = property.getFullyQualifiedTypeName();
nextModel = property;
// For primitive optional properties, keep the parent model as the
// current scope so that named variables (e.g. {{age}}) resolve
// against the parent class. The property is stashed separately
// so that {{this}} can still resolve to the primitive type.
nextModel = currentModel;
} else {
thing.elementType = property.getFullyQualifiedTypeName();
nextModel = parameters.introspector.getClassDeclaration(thing.elementType);
Expand All @@ -335,12 +339,13 @@ class TypeVisitor {
templateMarkModelManager:parameters.templateMarkModelManager,
introspector:parameters.introspector,
model:nextModel,
primitiveProperty: property.isPrimitive() ? property : null,
kind:parameters.kind
}, 'whenSome');
TypeVisitor.visitChildren(this, thing, {
templateMarkModelManager:parameters.templateMarkModelManager,
introspector:parameters.introspector,
model:null,
model:currentModel,
kind:parameters.kind
}, 'whenNone');
}
Expand Down
139 changes: 139 additions & 0 deletions packages/markdown-template/test/TemplateMarkTransformer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,24 @@ concept Thing {
o String[] items
}`;

const CONDITIONAL_MODEL = `
namespace test@1.0.0
@template
concept TemplateData {
o Integer age optional
o String name
o Boolean isActive optional
}`;

const OPTIONAL_MODEL = `
namespace test@1.0.0
@template
concept TemplateData {
o Integer age optional
o String middleName optional
o Boolean active optional
}`;

describe('#TemplateMarkTransformer', () => {
describe('#tokensToMarkdownTemplate', () => {
it('should handle join with type, style and locale', async () => {
Expand Down Expand Up @@ -92,4 +110,125 @@ describe('#TemplateMarkTransformer', () => {
(joinNode.foo === undefined).should.be.true;
});
});

describe('#conditional blocks with variables', () => {
it('should allow named variables inside #if blocks', async () => {
const transformer = new TemplateMarkTransformer();
const modelManager = new ModelManager();
modelManager.addCTOModel(CONDITIONAL_MODEL);
const result = transformer.fromMarkdownTemplate(
{content: '{{#if age}}You are {{age}} years old.{{/if}}'},
modelManager, 'clause', {verbose: false}
);
result.should.not.be.null;
});

it('should allow multiple variables inside #if blocks', async () => {
const transformer = new TemplateMarkTransformer();
const modelManager = new ModelManager();
modelManager.addCTOModel(CONDITIONAL_MODEL);
const result = transformer.fromMarkdownTemplate(
{content: '{{#if isActive}}Hello {{name}}, you are {{age}} years old.{{/if}}'},
modelManager, 'clause', {verbose: false}
);
result.should.not.be.null;
});

it('should reject unknown variables inside #if blocks', async () => {
const transformer = new TemplateMarkTransformer();
const modelManager = new ModelManager();
modelManager.addCTOModel(CONDITIONAL_MODEL);
(() => transformer.fromMarkdownTemplate(
{content: '{{#if age}}You are {{unknown}} years old.{{/if}}'},
modelManager, 'clause', {verbose: false}
)).should.throw();
});

it('should allow variables inside {{else}} branch of #if blocks', async () => {
const transformer = new TemplateMarkTransformer();
const modelManager = new ModelManager();
modelManager.addCTOModel(CONDITIONAL_MODEL);
const result = transformer.fromMarkdownTemplate(
{content: '{{#if isActive}}Hello {{name}}.{{else}}Goodbye {{name}}.{{/if}}'},
modelManager, 'clause', {verbose: false}
);
result.should.not.be.null;
});

it('should reject unknown variables inside {{else}} branch of #if blocks', async () => {
const transformer = new TemplateMarkTransformer();
const modelManager = new ModelManager();
modelManager.addCTOModel(CONDITIONAL_MODEL);
(() => transformer.fromMarkdownTemplate(
{content: '{{#if isActive}}Hello {{name}}.{{else}}Goodbye {{unknown}}.{{/if}}'},
modelManager, 'clause', {verbose: false}
)).should.throw();
});
});

describe('#optional blocks with variables', () => {
it('should allow named variables inside #optional blocks with primitive types', async () => {
const transformer = new TemplateMarkTransformer();
const modelManager = new ModelManager();
modelManager.addCTOModel(OPTIONAL_MODEL);
const result = transformer.fromMarkdownTemplate(
{content: '{{#optional age}}You are {{age}} years old.{{else}}Age unknown.{{/optional}}'},
modelManager, 'clause', {verbose: false}
);
result.should.not.be.null;
});

it('should allow different variables inside #optional blocks', async () => {
const transformer = new TemplateMarkTransformer();
const modelManager = new ModelManager();
modelManager.addCTOModel(OPTIONAL_MODEL);
const result = transformer.fromMarkdownTemplate(
{content: '{{#optional middleName}}Middle name: {{middleName}}{{else}}No middle name.{{/optional}}'},
modelManager, 'clause', {verbose: false}
);
result.should.not.be.null;
});

it('should reject unknown variables inside #optional blocks', async () => {
const transformer = new TemplateMarkTransformer();
const modelManager = new ModelManager();
modelManager.addCTOModel(OPTIONAL_MODEL);
(() => transformer.fromMarkdownTemplate(
{content: '{{#optional age}}You are {{unknown}} years old.{{else}}Nope.{{/optional}}'},
modelManager, 'clause', {verbose: false}
)).should.throw();
});

it('should allow variables inside {{else}} branch of #optional blocks', async () => {
const transformer = new TemplateMarkTransformer();
const modelManager = new ModelManager();
modelManager.addCTOModel(OPTIONAL_MODEL);
const result = transformer.fromMarkdownTemplate(
{content: '{{#optional age}}You are {{age}} years old.{{else}}Active: {{active}}.{{/optional}}'},
modelManager, 'clause', {verbose: false}
);
result.should.not.be.null;
});

it('should reject unknown variables inside {{else}} branch of #optional blocks', async () => {
const transformer = new TemplateMarkTransformer();
const modelManager = new ModelManager();
modelManager.addCTOModel(OPTIONAL_MODEL);
(() => transformer.fromMarkdownTemplate(
{content: '{{#optional age}}You are {{age}}.{{else}}Unknown: {{bogus}}.{{/optional}}'},
modelManager, 'clause', {verbose: false}
)).should.throw();
});

it('should allow {{this}} inside primitive #optional blocks', async () => {
const transformer = new TemplateMarkTransformer();
const modelManager = new ModelManager();
modelManager.addCTOModel(OPTIONAL_MODEL);
const result = transformer.fromMarkdownTemplate(
{content: '{{#optional age}}Age is {{this}}.{{else}}No age.{{/optional}}'},
modelManager, 'clause', {verbose: false}
);
result.should.not.be.null;
});
});
});