Skip to content

Commit 4ebe6a1

Browse files
committed
wip
1 parent 2fc44f3 commit 4ebe6a1

9 files changed

Lines changed: 221 additions & 4 deletions

File tree

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
const _ = require('lodash');
2+
const path = require('path');
3+
const spec = require('../specs');
4+
const {versions, normalizePath} = require('../utils');
5+
6+
const ruleCode = 'GS130-NO-RECURSIVE-LAYOUT';
7+
const layoutPattern = /{{!<\s+([A-Za-z0-9._/-]+)\s*}}/g;
8+
9+
function stripLeadingSlash(filePath) {
10+
return filePath.replace(/^\/+/, '');
11+
}
12+
13+
function ensureExtension(layoutPath) {
14+
if (path.posix.extname(layoutPath)) {
15+
return layoutPath;
16+
}
17+
18+
return `${layoutPath}.hbs`;
19+
}
20+
21+
function resolveLayoutPath(sourceFile, layoutName) {
22+
const source = normalizePath(sourceFile);
23+
const layout = ensureExtension(normalizePath(layoutName));
24+
25+
if (layout.startsWith('./') || layout.startsWith('../')) {
26+
return stripLeadingSlash(path.posix.normalize(path.posix.join(path.posix.dirname(source), layout)));
27+
}
28+
29+
return stripLeadingSlash(path.posix.normalize(layout));
30+
}
31+
32+
function getLocation(source, index) {
33+
const lines = source.slice(0, index).split('\n');
34+
35+
return {
36+
line: lines.length,
37+
column: lines[lines.length - 1].length
38+
};
39+
}
40+
41+
function getLayoutReferences(themeFile) {
42+
const source = normalizePath(themeFile.normalizedFile || themeFile.file);
43+
const content = themeFile.content || '';
44+
const references = [];
45+
let match;
46+
47+
layoutPattern.lastIndex = 0;
48+
49+
while ((match = layoutPattern.exec(content)) !== null) {
50+
references.push({
51+
source,
52+
target: resolveLayoutPath(source, match[1]),
53+
location: getLocation(content, match.index)
54+
});
55+
}
56+
57+
return references;
58+
}
59+
60+
function buildInheritanceGraph(theme) {
61+
const hbsFiles = theme.files.filter(file => file.ext === '.hbs');
62+
const existingFiles = new Set(hbsFiles.map(file => normalizePath(file.normalizedFile || file.file)));
63+
const graph = new Map();
64+
65+
hbsFiles.forEach((themeFile) => {
66+
getLayoutReferences(themeFile).forEach((reference) => {
67+
if (!existingFiles.has(reference.target)) {
68+
return;
69+
}
70+
71+
if (!graph.has(reference.source)) {
72+
graph.set(reference.source, []);
73+
}
74+
75+
graph.get(reference.source).push(reference);
76+
});
77+
});
78+
79+
return graph;
80+
}
81+
82+
function getCyclePath(stack, target) {
83+
const targetIndex = stack.indexOf(target);
84+
return stack.slice(targetIndex).concat(target).join(' -> ');
85+
}
86+
87+
function getRecursiveLayoutFailures(graph) {
88+
const visited = new Set();
89+
const visiting = new Set();
90+
const stack = [];
91+
const reportedCycles = new Set();
92+
const failures = [];
93+
94+
function visit(file) {
95+
visiting.add(file);
96+
stack.push(file);
97+
98+
const references = graph.get(file) || [];
99+
100+
references.forEach((reference) => {
101+
if (visiting.has(reference.target)) {
102+
const cyclePath = getCyclePath(stack, reference.target);
103+
104+
if (!reportedCycles.has(cyclePath)) {
105+
reportedCycles.add(cyclePath);
106+
107+
failures.push({
108+
ref: reference.source,
109+
line: reference.location.line,
110+
column: reference.location.column,
111+
message: `Recursive layout inheritance detected: ${cyclePath}`
112+
});
113+
}
114+
115+
return;
116+
}
117+
118+
if (!visited.has(reference.target)) {
119+
visit(reference.target);
120+
}
121+
});
122+
123+
stack.pop();
124+
visiting.delete(file);
125+
visited.add(file);
126+
}
127+
128+
graph.forEach((references, file) => { // eslint-disable-line no-unused-vars
129+
if (!visited.has(file)) {
130+
visit(file);
131+
}
132+
});
133+
134+
return failures;
135+
}
136+
137+
const checkTemplateInheritance = function checkTemplateInheritance(theme, options) {
138+
const checkVersion = _.get(options, 'checkVersion', versions.default);
139+
const ruleSet = spec.get([checkVersion]);
140+
141+
if (!ruleSet.rules[ruleCode]) {
142+
return theme;
143+
}
144+
145+
const failures = getRecursiveLayoutFailures(buildInheritanceGraph(theme));
146+
147+
if (failures.length > 0) {
148+
theme.results.fail[ruleCode] = {failures};
149+
} else {
150+
theme.results.pass.push(ruleCode);
151+
}
152+
153+
return theme;
154+
};
155+
156+
module.exports = checkTemplateInheritance;

lib/specs/v6.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,12 @@ let rules = {
4848
details: 'AMP support was removed in Ghost 6.0. Remove AMP templates and use responsive design instead.',
4949
// Matches <html amp> or <html ⚡>, with or without other attributes mixed in
5050
regex: /<html\s+(?:amp|)(?:\s|>)|<html\s+[^>]*\s(?:amp|)(?:\s|>)/i
51+
},
52+
'GS130-NO-RECURSIVE-LAYOUT': {
53+
level: 'error',
54+
fatal: true,
55+
rule: 'Templates must not recursively inherit layouts',
56+
details: oneLineTrim`Remove recursive layout inheritance such as <code>{{!&lt; default}}</code> from <code>default.hbs</code>. A template cannot inherit from itself, directly or through another layout, because it can cause rendering to recurse indefinitely.`
5157
}
5258
};
5359

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
var should = require('should'), // eslint-disable-line no-unused-vars
2+
utils = require('./utils'),
3+
thisCheck = require('../lib/checks/130-template-inheritance');
4+
5+
const ruleCode = 'GS130-NO-RECURSIVE-LAYOUT';
6+
7+
describe('130 Template inheritance', function () {
8+
it('detects default.hbs inheriting from default.hbs in v6', async function () {
9+
const output = await utils.testCheck(thisCheck, '130-template-inheritance/recursive-default', {checkVersion: 'v6'});
10+
11+
output.should.be.a.ValidThemeObject();
12+
Object.keys(output.results.fail).should.eql([ruleCode]);
13+
14+
const failure = output.results.fail[ruleCode].failures[0];
15+
failure.ref.should.eql('default.hbs');
16+
failure.message.should.containEql('default.hbs -> default.hbs');
17+
});
18+
19+
it('does not run for v5', async function () {
20+
const output = await utils.testCheck(thisCheck, '130-template-inheritance/recursive-default', {checkVersion: 'v5'});
21+
22+
output.should.be.a.ValidThemeObject();
23+
output.results.fail.should.be.an.Object().which.is.empty();
24+
output.results.pass.should.not.containEql(ruleCode);
25+
});
26+
27+
it('allows a template to inherit from default.hbs in v6', async function () {
28+
const output = await utils.testCheck(thisCheck, '130-template-inheritance/valid-default', {checkVersion: 'v6'});
29+
30+
output.should.be.a.ValidThemeObject();
31+
output.results.fail.should.be.an.Object().which.is.empty();
32+
output.results.pass.should.containEql(ruleCode);
33+
});
34+
35+
it('detects indirect recursive layout inheritance in v6', async function () {
36+
const output = await utils.testCheck(thisCheck, '130-template-inheritance/indirect-cycle', {checkVersion: 'v6'});
37+
38+
output.should.be.a.ValidThemeObject();
39+
Object.keys(output.results.fail).should.eql([ruleCode]);
40+
41+
const failure = output.results.fail[ruleCode].failures[0];
42+
failure.message.should.containEql('default.hbs');
43+
failure.message.should.containEql('layouts/base.hbs');
44+
});
45+
});

test/checker.test.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ describe('Checker', function () {
476476
{file: 'README.md', normalizedFile: 'README.md', ext: '.md', symlink: false}
477477
]);
478478

479-
theme.results.pass.should.be.an.Array().with.lengthOf(123);
479+
theme.results.pass.should.be.an.Array().with.lengthOf(124);
480480
theme.results.pass.should.eql([
481481
'GS001-DEPR-PURL',
482482
'GS001-DEPR-MD',
@@ -600,7 +600,8 @@ describe('Checker', function () {
600600
'GS090-NO-TIER-BENEFIT-AS-OBJECT',
601601
'GS090-NO-LIMIT-ALL-IN-GET-HELPER',
602602
'GS090-NO-LIMIT-OVER-100-IN-GET-HELPER',
603-
'GS120-NO-UNKNOWN-GLOBALS'
603+
'GS120-NO-UNKNOWN-GLOBALS',
604+
'GS130-NO-RECURSIVE-LAYOUT'
604605
]);
605606

606607
theme.results.fail.should.be.an.Object().with.keys(
@@ -636,7 +637,7 @@ describe('Checker', function () {
636637
]);
637638

638639
// Short version of test above
639-
theme.results.pass.should.be.an.Array().with.lengthOf(123);
640+
theme.results.pass.should.be.an.Array().with.lengthOf(124);
640641
theme.checkedVersion.should.equal('6.x');
641642
done();
642643
}).catch(done);
@@ -652,7 +653,7 @@ describe('Checker', function () {
652653
]);
653654

654655
// Should default to v6 behavior
655-
theme.results.pass.should.be.an.Array().with.lengthOf(123);
656+
theme.results.pass.should.be.an.Array().with.lengthOf(124);
656657
theme.checkedVersion.should.equal('6.x');
657658
done();
658659
}).catch(done);
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
{{!< layouts/base}}
2+
{{{body}}}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
{{!< default}}
2+
{{{body}}}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
{{!< default}}
2+
{{{body}}}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{{{body}}}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
{{!< default}}
2+
<h1>Home</h1>

0 commit comments

Comments
 (0)