Skip to content

Commit 00d941f

Browse files
SleeplessByteSleeplessByte
authored
Improve lasagna analyzer (#83)
1 parent 3663375 commit 00d941f

File tree

8 files changed

+298
-187
lines changed

8 files changed

+298
-187
lines changed

‎CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
# Changelog
22

3+
## 0.11.2
4+
5+
- Improve `concept/lasagna` analysis
6+
- Add various generic comments
7+
38
## 0.11.1
49

510
- Remove `console.log` from EarlyFinalisation code path

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@exercism/javascript-analyzer",
3-
"version": "0.11.1",
3+
"version": "0.11.2",
44
"description": "Exercism analyzer for javascript",
55
"repository": "https://github.com/exercism/javascript-analyzer",
66
"author": "Derk-Jan Karrenbeld <[email protected]>",

‎src/analyzers/concept/lasagna/LasagnaSolution.ts

Lines changed: 149 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -1,141 +1,54 @@
11
import {
22
AstParser,
3-
AstTraverser,
43
ExtractedExport,
54
ExtractedFunction,
65
extractExports,
76
extractFunctions,
7+
findFirst,
8+
findLiteral,
9+
findMemberCall,
10+
findRawLiteral,
811
findTopLevelConstants,
9-
guardBinaryExpression,
1012
guardCallExpression,
1113
guardIdentifier,
1214
guardLiteral,
15+
IdentifierWithName,
1316
ProgramConstant,
17+
SpecificFunctionCall,
1418
StructureError,
1519
traverse,
1620
} from '@exercism/static-analysis'
17-
import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/typescript-estree'
21+
import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/typescript-estree'
1822
import { readFileSync } from 'fs'
23+
import path from 'path'
1924
import { assertNamedExport } from '~src/asserts/assert_named_export'
2025
import { assertNamedFunction } from '~src/asserts/assert_named_function'
26+
import { assertPublicApi } from '../../../asserts/assert_public_api'
27+
import { assertPublicConstant } from '../../../asserts/assert_public_constant'
2128
import { Source } from '../../SourceImpl'
2229
import { parameterName } from '../../utils/extract_parameter'
23-
import path from 'path'
2430

25-
const REMAINING_MINUTES_IN_OVEN = 'remainingMinutesInOven'
26-
const PREPARATION_TIME_IN_MINUTES = 'preparationTimeInMinutes'
27-
const TOTAL_TIME_IN_MINUTES = 'totalTimeInMinutes'
28-
const EXPECTED_MINUTES_IN_OVEN = 'EXPECTED_MINUTES_IN_OVEN'
31+
export const REMAINING_MINUTES_IN_OVEN = 'remainingMinutesInOven'
32+
export const PREPARATION_TIME_IN_MINUTES = 'preparationTimeInMinutes'
33+
export const TOTAL_TIME_IN_MINUTES = 'totalTimeInMinutes'
34+
export const EXPECTED_MINUTES_IN_OVEN = 'EXPECTED_MINUTES_IN_OVEN'
2935

30-
export class NoPublicConstantError extends StructureError {
31-
constructor(public name: string) {
32-
super(`Expected an exported constant named ${name}, but did not find it.`)
36+
class RemainingMinutesInOven {
37+
public readonly parameter: string
3338

34-
Error.captureStackTrace(this, this.constructor)
39+
constructor(private readonly implementation: ExtractedFunction) {
40+
this.parameter = parameterName(implementation.params[0])
3541
}
36-
}
3742

38-
function assertPublicApi(
39-
exported: string,
40-
exports: ExtractedExport[],
41-
functions: ExtractedFunction[]
42-
): ExtractedFunction {
43-
const namedExport = assertNamedExport(exported, exports)
44-
return assertNamedFunction(namedExport.local, functions)
45-
}
46-
47-
function assertPublicConstant(
48-
exported: string,
49-
exports: ExtractedExport[],
50-
root: TSESTree.Node
51-
): ProgramConstant {
52-
const namedExport = assertNamedExport(exported, exports)
53-
const result = findTopLevelConstants(root, ['let', 'const', 'var']).find(
54-
(constant) =>
55-
guardIdentifier(constant.id) && constant.id.name === namedExport.name
56-
)
57-
58-
if (!result) {
59-
throw new NoPublicConstantError(exported)
43+
public traverse(options: Parameters<typeof traverse>[1]): void {
44+
traverse(this.implementation.body, options)
6045
}
6146

62-
return result
63-
}
64-
65-
export class LasagnaSolution {
66-
private readonly source: Source
67-
68-
private readonly remainingMinutesInOven: ExtractedFunction
69-
private readonly preparationTimeInMinutes: ExtractedFunction
70-
private readonly totalTimeInMinutes: ExtractedFunction
71-
private readonly expectedMinutesInOven: ProgramConstant
72-
73-
private exemplar!: Source
74-
75-
constructor(public readonly program: TSESTree.Program, source: string) {
76-
this.source = new Source(source)
77-
78-
const functions = extractFunctions(program)
79-
const exports = extractExports(program)
80-
81-
this.expectedMinutesInOven = assertPublicConstant(
82-
EXPECTED_MINUTES_IN_OVEN,
83-
exports,
84-
program
85-
)
86-
87-
this.remainingMinutesInOven = assertPublicApi(
88-
REMAINING_MINUTES_IN_OVEN,
89-
exports,
90-
functions
91-
)
92-
93-
this.preparationTimeInMinutes = assertPublicApi(
94-
PREPARATION_TIME_IN_MINUTES,
95-
exports,
96-
functions
97-
)
98-
99-
this.totalTimeInMinutes = assertPublicApi(
100-
TOTAL_TIME_IN_MINUTES,
101-
exports,
102-
functions
103-
)
104-
}
105-
106-
public readExemplar(directory: string): void {
107-
const configPath = path.join(directory, '.meta', 'config.json')
108-
const config = JSON.parse(readFileSync(configPath).toString())
109-
110-
const exemplarPath = path.join(directory, config.files.exemplar[0])
111-
this.exemplar = new Source(readFileSync(exemplarPath).toString())
112-
}
113-
114-
public get isExemplar(): boolean {
115-
const sourceAst = AstParser.REPRESENTER.parseSync(this.source.toString())
116-
const exemplarAst = AstParser.REPRESENTER.parseSync(
117-
this.exemplar.toString()
118-
)
119-
120-
return (
121-
JSON.stringify(sourceAst[0].program) ===
122-
JSON.stringify(exemplarAst[0].program)
123-
)
124-
}
125-
126-
public get hasConstantDeclaredAsConst(): boolean {
127-
return this.expectedMinutesInOven.kind === 'const'
128-
}
129-
130-
public get hasOptimalConstant(): boolean {
131-
return guardLiteral(this.expectedMinutesInOven.init!, 40)
132-
}
133-
134-
public get hasOptimalRemainingMinutesInOven(): boolean {
135-
const parameter = parameterName(this.remainingMinutesInOven.params[0])
47+
public get isOptimal(): boolean {
48+
const parameter = this.parameter
13649

13750
let foundSuboptimalNode = false
138-
traverse(this.remainingMinutesInOven.body, {
51+
this.traverse({
13952
enter() {
14053
foundSuboptimalNode = true
14154
},
@@ -170,11 +83,36 @@ export class LasagnaSolution {
17083
return !foundSuboptimalNode
17184
}
17285

173-
public get hasOptimalPreparationTimeInMinutes(): boolean {
174-
const parameter = parameterName(this.preparationTimeInMinutes.params[0])
86+
public get hasReplacableLiteral(): boolean {
87+
const hasConstantLiteral = findFirst(
88+
this.implementation.body,
89+
(node): node is IdentifierWithName<typeof EXPECTED_MINUTES_IN_OVEN> =>
90+
guardIdentifier(node, EXPECTED_MINUTES_IN_OVEN)
91+
)
92+
const hasLiteral = findRawLiteral(this.implementation.body, '40')
93+
return Boolean(hasLiteral && !hasConstantLiteral)
94+
}
95+
}
96+
97+
class TotalTimeInMinutes {
98+
public readonly numberOfLayers: string
99+
public readonly actualMinutesInOven: string
100+
101+
constructor(private readonly implementation: ExtractedFunction) {
102+
this.numberOfLayers = parameterName(this.implementation.params[0])
103+
this.actualMinutesInOven = parameterName(this.implementation.params[1])
104+
}
105+
106+
public traverse(options: Parameters<typeof traverse>[1]): void {
107+
traverse(this.implementation.body, options)
108+
}
109+
110+
public get isOptimal(): boolean {
111+
const numberOfLayers = this.numberOfLayers
112+
const actualMinutesInOven = this.actualMinutesInOven
175113

176114
let foundSuboptimalNode = false
177-
traverse(this.preparationTimeInMinutes.body, {
115+
this.traverse({
178116
enter() {
179117
foundSuboptimalNode = true
180118
},
@@ -190,14 +128,17 @@ export class LasagnaSolution {
190128
},
191129

192130
[AST_NODE_TYPES.BinaryExpression](node) {
193-
// TODO look for top-level constant and use that name instead.
194131
foundSuboptimalNode =
195-
node.operator !== '*' ||
132+
node.operator !== '+' ||
196133
!(
197-
(guardIdentifier(node.left, parameter) &&
198-
guardIdentifier(node.right, 'PREPARATION_MINUTES_PER_LAYER')) ||
199-
(guardIdentifier(node.right, parameter) &&
200-
guardIdentifier(node.left, 'PREPARATION_MINUTES_PER_LAYER'))
134+
(guardCallExpression(node.left, PREPARATION_TIME_IN_MINUTES) &&
135+
node.left.arguments.length === 1 &&
136+
guardIdentifier(node.left.arguments[0], numberOfLayers) &&
137+
guardIdentifier(node.right, actualMinutesInOven)) ||
138+
(guardCallExpression(node.right, PREPARATION_TIME_IN_MINUTES) &&
139+
node.right.arguments.length === 1 &&
140+
guardIdentifier(node.right.arguments[0], numberOfLayers) &&
141+
guardIdentifier(node.left, actualMinutesInOven))
201142
)
202143
this.skip()
203144
},
@@ -212,12 +153,91 @@ export class LasagnaSolution {
212153
return !foundSuboptimalNode
213154
}
214155

215-
public get hasOptimalTotalTimeInMinutes(): boolean {
216-
const numberOfLayers = parameterName(this.totalTimeInMinutes.params[0])
217-
const actualMinutesInOven = parameterName(this.totalTimeInMinutes.params[1])
156+
public get hasCallToPreparationTime(): boolean {
157+
return !!findFirst(
158+
this.implementation.body,
159+
(
160+
node
161+
): node is SpecificFunctionCall<typeof PREPARATION_TIME_IN_MINUTES> =>
162+
guardCallExpression(node, PREPARATION_TIME_IN_MINUTES)
163+
)
164+
}
165+
}
166+
167+
export class LasagnaSolution {
168+
private readonly source: Source
169+
170+
public readonly remainingMinutesInOven: RemainingMinutesInOven
171+
public readonly totalTimeInMinutes: TotalTimeInMinutes
172+
private readonly preparationTimeInMinutes: ExtractedFunction
173+
private readonly expectedMinutesInOven: ProgramConstant
174+
175+
private exemplar!: Source
176+
177+
constructor(public readonly program: TSESTree.Program, source: string) {
178+
this.source = new Source(source)
179+
180+
const functions = extractFunctions(program)
181+
const exports = extractExports(program)
182+
183+
this.expectedMinutesInOven = assertPublicConstant(
184+
EXPECTED_MINUTES_IN_OVEN,
185+
exports,
186+
program
187+
)
188+
189+
this.preparationTimeInMinutes = assertPublicApi(
190+
PREPARATION_TIME_IN_MINUTES,
191+
exports,
192+
functions
193+
)
194+
195+
this.remainingMinutesInOven = new RemainingMinutesInOven(
196+
assertPublicApi(REMAINING_MINUTES_IN_OVEN, exports, functions)
197+
)
198+
199+
this.totalTimeInMinutes = new TotalTimeInMinutes(
200+
assertPublicApi(TOTAL_TIME_IN_MINUTES, exports, functions)
201+
)
202+
}
203+
204+
public readExemplar(directory: string): void {
205+
const configPath = path.join(directory, '.meta', 'config.json')
206+
const config = JSON.parse(readFileSync(configPath).toString())
207+
208+
const exemplarPath = path.join(directory, config.files.exemplar[0])
209+
this.exemplar = new Source(readFileSync(exemplarPath).toString())
210+
}
211+
212+
public get isExemplar(): boolean {
213+
const sourceAst = AstParser.REPRESENTER.parseSync(this.source.toString())
214+
const exemplarAst = AstParser.REPRESENTER.parseSync(
215+
this.exemplar.toString()
216+
)
217+
218+
return (
219+
JSON.stringify(sourceAst[0].program) ===
220+
JSON.stringify(exemplarAst[0].program)
221+
)
222+
}
223+
224+
public get constantKind(): ProgramConstant['kind'] {
225+
return this.expectedMinutesInOven.kind
226+
}
227+
228+
public get hasConstantDeclaredAsConst(): boolean {
229+
return this.constantKind === 'const'
230+
}
231+
232+
public get hasOptimalConstant(): boolean {
233+
return guardLiteral(this.expectedMinutesInOven.init!, 40)
234+
}
235+
236+
public get hasOptimalPreparationTimeInMinutes(): boolean {
237+
const parameter = parameterName(this.preparationTimeInMinutes.params[0])
218238

219239
let foundSuboptimalNode = false
220-
traverse(this.totalTimeInMinutes.body, {
240+
traverse(this.preparationTimeInMinutes.body, {
221241
enter() {
222242
foundSuboptimalNode = true
223243
},
@@ -233,17 +253,14 @@ export class LasagnaSolution {
233253
},
234254

235255
[AST_NODE_TYPES.BinaryExpression](node) {
256+
// TODO look for top-level constant and use that name instead.
236257
foundSuboptimalNode =
237-
node.operator !== '+' ||
258+
node.operator !== '*' ||
238259
!(
239-
(guardCallExpression(node.left, PREPARATION_TIME_IN_MINUTES) &&
240-
node.left.arguments.length === 1 &&
241-
guardIdentifier(node.left.arguments[0], numberOfLayers) &&
242-
guardIdentifier(node.right, actualMinutesInOven)) ||
243-
(guardCallExpression(node.right, PREPARATION_TIME_IN_MINUTES) &&
244-
node.right.arguments.length === 1 &&
245-
guardIdentifier(node.right.arguments[0], numberOfLayers) &&
246-
guardIdentifier(node.left, actualMinutesInOven))
260+
(guardIdentifier(node.left, parameter) &&
261+
guardIdentifier(node.right, 'PREPARATION_MINUTES_PER_LAYER')) ||
262+
(guardIdentifier(node.right, parameter) &&
263+
guardIdentifier(node.left, 'PREPARATION_MINUTES_PER_LAYER'))
247264
)
248265
this.skip()
249266
},

0 commit comments

Comments
 (0)