Skip to content

Commit 9f0e585

Browse files
resistor-color-duo: allow non-optimal constants during analysis (#111)
* Fixes #101 Allow for non-optimal constants. We currently don't provided messages about the constants themselves, so instead, when not checking for the optimal solution, continue with sub-optimal constants as well. Before: ```javascript export const COLORS = [ 'black', 'brown', 'red', 'orange', 'yellow', 'green', 'blue', 'violet', 'grey', 'White', ] // => Helper is not optimal ``` The helper was marked as non-optimal, where it was actually the referenced constant that was not optimal. * Read prettier from package.json * Remove explicit out-of-sync prettier from workflows * Apply formatting
1 parent 90d27cb commit 9f0e585

File tree

23 files changed

+220
-491
lines changed

23 files changed

+220
-491
lines changed

.github/workflows/format-code.yml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,6 @@ jobs:
6262
6363
- name: 'Format code'
6464
run: ./bin/format.sh
65-
env:
66-
EXERCISM_PRETTIER_VERSION: '2.2.1'
6765

6866
- name: 'Commit formatted code'
6967
run: |

.github/workflows/verify-code-formatting.yml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,3 @@ jobs:
1414

1515
- name: 'Verify formatting of all files'
1616
run: ./bin/check-formatting.sh
17-
env:
18-
EXERCISM_PRETTIER_VERSION: '2.2.1'

CHANGELOG.md

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

3+
## 0.15.0
4+
5+
- Allow non-optimal constants in `resistor-color-duo`
6+
37
## 0.14.0
48

59
- Update dependencies

README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,21 @@ yarn install
1515

1616
You'll need at least Node LTS for this to work.
1717

18-
```
18+
```bash
1919
yarn build
2020
```
2121

2222
## Usage
2323

2424
You can run this either via `yarn`:
2525

26-
```
26+
```bash
2727
yarn analyze:bat --debug --console two-fer ~/path/to/solution/folder
2828
```
2929

3030
Or directly via the provided shell script:
3131

32-
```
32+
```bash
3333
./bin/analyze.sh --debug --console two-fer ~/path/to/solution/folder
3434
```
3535

bin/check-formatting.sh

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
11
#!/bin/bash
22

3+
if [ -z "$EXERCISM_PRETTIER_VERSION" ]; then
4+
echo "Pulling prettier version from package.json"
5+
EXERCISM_PRETTIER_VERSION=$(npm list prettier | grep -Po '.*prettier@\K.*')
6+
fi
7+
8+
if [ -z "$EXERCISM_PRETTIER_VERSION" ]; then
9+
echo "This script requires the EXERCISM_PRETTIER_VERSION variable to work."
10+
echo "Please see https://github.com/exercism/v3/blob/master/docs/maintainers/style-guide.md for guidance."
11+
exit 1
12+
else
13+
echo "Running format with prettier@$EXERCISM_PRETTIER_VERSION"
14+
fi
15+
316
npx "prettier@$EXERCISM_PRETTIER_VERSION" --check "**/*.{js,jsx,ts,tsx,css,sass,scss,html,json,md,yml}"

bin/format.sh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,16 @@
11
#!/usr/bin/env bash
22

3+
if [ -z "$EXERCISM_PRETTIER_VERSION" ]; then
4+
echo "Pulling prettier version from package.json"
5+
EXERCISM_PRETTIER_VERSION=$(npm list prettier | grep -Po '.*prettier@\K.*')
6+
fi
7+
38
if [ -z "$EXERCISM_PRETTIER_VERSION" ]; then
49
echo "This script requires the EXERCISM_PRETTIER_VERSION variable to work."
510
echo "Please see https://github.com/exercism/v3/blob/master/docs/maintainers/style-guide.md for guidance."
611
exit 1
12+
else
13+
echo "Running format with prettier@$EXERCISM_PRETTIER_VERSION"
714
fi
815

916
npx "prettier@$EXERCISM_PRETTIER_VERSION" --write "**/*.{js,jsx,ts,tsx,css,sass,scss,html,json,md,yml}"

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@exercism/javascript-analyzer",
3-
"version": "0.14.0",
3+
"version": "0.15.0",
44
"description": "Exercism analyzer for javascript",
55
"repository": "https://github.com/exercism/javascript-analyzer",
66
"author": "Derk-Jan Karrenbeld <[email protected]>",
@@ -51,6 +51,7 @@
5151
"eslint-plugin-import": "^2.24.2",
5252
"eslint-plugin-jest": "^24.4.0",
5353
"jest": "^27.1.0",
54+
"prettier": "^2.3.2",
5455
"rimraf": "^3.0.2",
5556
"shelljs": "^0.8.4"
5657
},

src/analyzers/practice/gigasecond/GigasecondSolution.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,8 @@ class Constant {
6868
return !!this._memoized['isOptimisedComprehension']
6969
}
7070

71-
return (this._memoized[
72-
'isOptimisedComprehension'
73-
] = isOptimisedComprehension(init))
71+
return (this._memoized['isOptimisedComprehension'] =
72+
isOptimisedComprehension(init))
7473
}
7574

7675
public get isLargeNumberLiteral(): boolean {
@@ -299,13 +298,13 @@ export class GigasecondSolution {
299298
this.mainExport = assertNamedExport(EXPECTED_EXPORT, exports)
300299

301300
// All constants at the top level that are _not_ the main method
302-
this.fileConstants = findTopLevelConstants(program, ([
301+
this.fileConstants = findTopLevelConstants(program, [
303302
'let',
304303
'const',
305304
'var',
306305

307306
// TODO upstream bug
308-
] as unknown) as ['let']).filter(
307+
] as unknown as ['let']).filter(
309308
(declaration): boolean =>
310309
declaration &&
311310
guardIdentifier(declaration.id) &&

src/analyzers/practice/resistor-color-duo/ResistorColorDuoSolution.ts

Lines changed: 81 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { assertNamedExport } from '~src/asserts/assert_named_export'
2727
import { assertNamedFunction } from '~src/asserts/assert_named_function'
2828
import { extractSignature } from '~src/extracts/extract_declaration'
2929
import { extractNamedFunction } from '~src/extracts/extract_named_function'
30+
import { guardLiteralCaseInsensitive } from '../../utils/guard_literal_case_insensitive'
3031

3132
type Node = TSESTree.Node
3233
type Program = TSESTree.Program
@@ -129,6 +130,36 @@ class Constant {
129130
return false
130131
}
131132

133+
public get isNonOptimalArray(): boolean {
134+
const { init } = this.constant
135+
136+
if (!init) {
137+
return false
138+
}
139+
140+
const literals = [
141+
'black',
142+
'brown',
143+
'red',
144+
'orange',
145+
'yellow',
146+
'green',
147+
'blue',
148+
'violet',
149+
'grey',
150+
'white',
151+
]
152+
153+
if (init.type === AST_NODE_TYPES.ArrayExpression) {
154+
// Each literal needs to be present, and needs to be present exactly in this order
155+
return init.elements.every((value, index): boolean =>
156+
guardLiteralCaseInsensitive(value, literals[index])
157+
)
158+
}
159+
160+
return false
161+
}
162+
132163
public get isObjectToArray(): boolean {
133164
const { init } = this.constant
134165

@@ -179,6 +210,39 @@ class Constant {
179210
})
180211
}
181212

213+
public isNonOptimalObject(
214+
node: ExtractedVariable | undefined = this.constant
215+
): boolean {
216+
if (!node || !node.init) {
217+
return false
218+
}
219+
220+
if (node.init.type !== AST_NODE_TYPES.ObjectExpression) {
221+
return false
222+
}
223+
224+
const keys = [
225+
'black',
226+
'brown',
227+
'red',
228+
'orange',
229+
'yellow',
230+
'green',
231+
'blue',
232+
'violet',
233+
'grey',
234+
'white',
235+
]
236+
237+
return node.init.properties.every((property, index): boolean => {
238+
return (
239+
property.type === AST_NODE_TYPES.Property &&
240+
guardLiteralCaseInsensitive(property.key, keys[index]) &&
241+
guardLiteral(property.value, index)
242+
)
243+
})
244+
}
245+
182246
/**
183247
* In the case that the top-level constant is constructed from Object.keys(argument),
184248
* this property holds the name of the "argument".
@@ -315,15 +379,19 @@ class Entry {
315379

316380
public get hasFor(): boolean {
317381
return (
318-
findFirst(this.body, (node): node is
319-
| TSESTree.ForInStatement
320-
| TSESTree.ForOfStatement
321-
| TSESTree.ForStatement =>
322-
[
323-
AST_NODE_TYPES.ForInStatement,
324-
AST_NODE_TYPES.ForOfStatement,
325-
AST_NODE_TYPES.ForStatement,
326-
].some((type) => type === node.type)
382+
findFirst(
383+
this.body,
384+
(
385+
node
386+
): node is
387+
| TSESTree.ForInStatement
388+
| TSESTree.ForOfStatement
389+
| TSESTree.ForStatement =>
390+
[
391+
AST_NODE_TYPES.ForInStatement,
392+
AST_NODE_TYPES.ForOfStatement,
393+
AST_NODE_TYPES.ForStatement,
394+
].some((type) => type === node.type)
327395
) !== undefined
328396
)
329397
}
@@ -961,7 +1029,7 @@ class Entry {
9611029
return false
9621030
}
9631031

964-
if (constant.isOptimalArray) {
1032+
if (constant.isOptimalArray || constant.isNonOptimalArray) {
9651033
logger.log('=> constant is optimal array')
9661034

9671035
// Only looking for:
@@ -977,7 +1045,7 @@ class Entry {
9771045
)
9781046
}
9791047

980-
if (constant.isOptimalObject()) {
1048+
if (constant.isOptimalObject() || constant.isNonOptimalObject()) {
9811049
logger.log('=> constant is optimal object')
9821050

9831051
// Only looking for:
@@ -1023,13 +1091,13 @@ export class ResistorColorDuoSolution {
10231091
this.mainExport = assertNamedExport(EXPECTED_EXPORT, exports)
10241092

10251093
// All constants at the top level that are _not_ the main method
1026-
this.fileConstants = findTopLevelConstants(program, ([
1094+
this.fileConstants = findTopLevelConstants(program, [
10271095
'let',
10281096
'const',
10291097
'var',
10301098

10311099
// TODO upstream bug
1032-
] as unknown) as ['let']).filter(
1100+
] as unknown as ['let']).filter(
10331101
(declaration): boolean =>
10341102
declaration &&
10351103
guardIdentifier(declaration.id) &&

src/analyzers/practice/resistor-color/ResistorColorSolution.ts

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -230,15 +230,19 @@ class Entry {
230230

231231
public get hasFor(): boolean {
232232
return (
233-
findFirst(this.body, (node): node is
234-
| TSESTree.ForInStatement
235-
| TSESTree.ForOfStatement
236-
| TSESTree.ForStatement =>
237-
[
238-
AST_NODE_TYPES.ForInStatement,
239-
AST_NODE_TYPES.ForOfStatement,
240-
AST_NODE_TYPES.ForStatement,
241-
].some((type) => type === node.type)
233+
findFirst(
234+
this.body,
235+
(
236+
node
237+
): node is
238+
| TSESTree.ForInStatement
239+
| TSESTree.ForOfStatement
240+
| TSESTree.ForStatement =>
241+
[
242+
AST_NODE_TYPES.ForInStatement,
243+
AST_NODE_TYPES.ForOfStatement,
244+
AST_NODE_TYPES.ForStatement,
245+
].some((type) => type === node.type)
242246
) !== undefined
243247
)
244248
}
@@ -400,13 +404,13 @@ export class ResistorColorSolution {
400404
}
401405

402406
// All constants at the top level that are _not_ the main method
403-
this.fileConstants = findTopLevelConstants(program, ([
407+
this.fileConstants = findTopLevelConstants(program, [
404408
'let',
405409
'const',
406410
'var',
407411

408412
// TODO: bug in upstream
409-
] as unknown) as ['let']).filter(
413+
] as unknown as ['let']).filter(
410414
(declaration): boolean =>
411415
declaration &&
412416
guardIdentifier(declaration.id) &&

0 commit comments

Comments
 (0)