Skip to content

Commit d6b4489

Browse files
jfmengelsjfmengels
committed
Improve error message when failing to update a suppression file
1 parent b56dfe4 commit d6b4489

File tree

13 files changed

+239
-5
lines changed

13 files changed

+239
-5
lines changed

‎CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
## [Unreleased]
44

5+
- Improve the error message when failing to update suppression files.
6+
57
## [2.11.0] - 2024-03-15
68

79
- Add an `offline` mode to prevent `elm-review` from making any HTTP requests. This is useful for CI environments that should not have access to the internet, where you only want to run `elm-review` without arguments.

‎lib/suppressed-errors.js

+17-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ module.exports = {
2626

2727
// WRITE
2828

29+
const orange = chalk.keyword('orange');
30+
2931
/**
3032
* @param {Options} options
3133
* @param {Array<SuppressedErrorsFile>} suppressionFiles
@@ -55,7 +57,21 @@ async function write(options, suppressionFiles) {
5557

5658
const previousContents = await FS.readFile(filePath).catch(() => '');
5759
if (previousContents !== newContents) {
58-
return FS.writeFile(filePath, newContents);
60+
return FS.writeFile(filePath, newContents)
61+
.catch(error => {
62+
const relativeFolderPath = path.relative(process.cwd(), options.suppressedErrorsFolder()) + '/';
63+
const relativeFilePath = path.relative(process.cwd(), filePath);
64+
throw new ErrorMessage.CustomError(
65+
'FAILED TO UPDATE SUPPRESSION FILE',
66+
// prettier-ignore
67+
`I tried updating the suppression file in the ${orange(relativeFolderPath)} folder, but failed to write to ${orange(relativeFilePath)}.
68+
69+
Please check that ${chalk.greenBright('elm-review')} has write permissions to that file and folder. In case it helps, here's the error I encountered:
70+
71+
${error.toString()}`,
72+
filePath
73+
);
74+
});
5975
}
6076

6177
return null;

‎test/project-with-suppressed-errors-no-write/elm.json

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"type": "application",
3+
"source-directories": [
4+
"src"
5+
],
6+
"elm-version": "0.19.1",
7+
"dependencies": {
8+
"direct": {
9+
"elm/browser": "1.0.2",
10+
"elm/core": "1.0.5",
11+
"elm/html": "1.0.0"
12+
},
13+
"indirect": {
14+
"elm/json": "1.1.3",
15+
"elm/time": "1.0.0",
16+
"elm/url": "1.0.0",
17+
"elm/virtual-dom": "1.0.2"
18+
}
19+
},
20+
"test-dependencies": {
21+
"direct": {},
22+
"indirect": {}
23+
}
24+
}

‎test/project-with-suppressed-errors-no-write/expected-NoUnused.Variables.json

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"version": 1,
3+
"automatically created by": "elm-review suppress",
4+
"learn more": "elm-review suppress --help",
5+
"suppressions": [
6+
{ "count": 2, "filePath": "src/Main.elm" }
7+
]
8+
}

‎test/project-with-suppressed-errors-no-write/fixed-elm.json

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"type": "application",
3+
"source-directories": [
4+
"src"
5+
],
6+
"elm-version": "0.19.1",
7+
"dependencies": {
8+
"direct": {
9+
"elm/browser": "1.0.2",
10+
"elm/core": "1.0.5",
11+
"elm/html": "1.0.0"
12+
},
13+
"indirect": {
14+
"elm/json": "1.1.3",
15+
"elm/time": "1.0.0",
16+
"elm/url": "1.0.0",
17+
"elm/virtual-dom": "1.0.2"
18+
}
19+
},
20+
"test-dependencies": {
21+
"direct": {},
22+
"indirect": {}
23+
}
24+
}

‎test/project-with-suppressed-errors-no-write/review/elm.json

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
{
2+
"type": "application",
3+
"source-directories": [
4+
"src"
5+
],
6+
"elm-version": "0.19.1",
7+
"dependencies": {
8+
"direct": {
9+
"elm/core": "1.0.5",
10+
"elm/json": "1.1.3",
11+
"elm/project-metadata-utils": "1.0.2",
12+
"jfmengels/elm-review": "2.13.0",
13+
"jfmengels/elm-review-unused": "1.1.29",
14+
"stil4m/elm-syntax": "7.2.9"
15+
},
16+
"indirect": {
17+
"elm/bytes": "1.0.8",
18+
"elm/html": "1.0.0",
19+
"elm/parser": "1.1.0",
20+
"elm/random": "1.0.0",
21+
"elm/time": "1.0.0",
22+
"elm/virtual-dom": "1.0.3",
23+
"elm-community/list-extra": "8.7.0",
24+
"elm-explorations/test": "2.1.1",
25+
"miniBill/elm-unicode": "1.0.3",
26+
"rtfeldman/elm-hex": "1.0.0",
27+
"stil4m/structured-writer": "1.0.3"
28+
}
29+
},
30+
"test-dependencies": {
31+
"direct": {},
32+
"indirect": {}
33+
}
34+
}

‎test/project-with-suppressed-errors-no-write/review/src/ReviewConfig.elm

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
module ReviewConfig exposing (config)
2+
3+
{-| Do not rename the ReviewConfig module or the config function, because
4+
`elm-review` will look for these.
5+
6+
To add packages that contain rules, add them to this review project using
7+
8+
`elm install author/packagename`
9+
10+
when inside the directory containing this file.
11+
12+
-}
13+
14+
15+
import NoUnused.Dependencies
16+
import NoUnused.Variables
17+
import Review.Rule exposing (Rule)
18+
19+
20+
config : List Rule
21+
config =
22+
[ NoUnused.Dependencies.rule
23+
, NoUnused.Variables.rule
24+
]

‎test/project-with-suppressed-errors-no-write/review/suppressed/NoUnused.Variables.json

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"version": 1,
3+
"automatically created by": "elm-review suppress",
4+
"learn more": "elm-review suppress --help",
5+
"suppressions": [
6+
{ "count": 2, "filePath": "src/Main.elm" },
7+
{ "count": 1, "filePath": "src/OtherFile.elm" }
8+
]
9+
}

‎test/project-with-suppressed-errors-no-write/src/Main.elm

+58
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
module Main exposing (main)
2+
3+
import Browser
4+
import Folder.Used exposing (one)
5+
import Html
6+
exposing
7+
( Html
8+
, button
9+
, div
10+
, -- h1 is unused
11+
h1
12+
, -- span is unused
13+
span
14+
, text
15+
)
16+
import Html.Events exposing (onClick)
17+
18+
19+
type alias Model =
20+
{ count : Int }
21+
22+
23+
initialModel : Model
24+
initialModel =
25+
{ count = 0 }
26+
27+
28+
type Msg
29+
= Increment
30+
| Decrement
31+
32+
33+
update : Msg -> Model -> Model
34+
update msg model =
35+
case msg of
36+
Increment ->
37+
{ model | count = model.count + one }
38+
39+
Decrement ->
40+
{ model | count = model.count - 1 }
41+
42+
43+
view : Model -> Html Msg
44+
view model =
45+
div []
46+
[ button [ onClick Increment ] [ text "+1" ]
47+
, div [] [ text <| String.fromInt model.count ]
48+
, button [ onClick Decrement ] [ text "-1" ]
49+
]
50+
51+
52+
main : Program () Model Msg
53+
main =
54+
Browser.sandbox
55+
{ init = initialModel
56+
, view = view
57+
, update = update
58+
}

‎test/project-with-suppressed-errors-no-write/src/OtherFile.elm

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
module OtherFile exposing (a)
2+
3+
4+
a =
5+
1

‎test/project-with-suppressed-errors-no-write/with-errors-OtherFile.elm

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
module OtherFile exposing (a)
2+
3+
4+
a =
5+
1
6+
7+
8+
b =
9+
2
10+
11+
12+
c =
13+
3

‎test/snapshots/suppress/write-failure.txt

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
-- FAILED TO UPDATE SUPPRESSION FILE -------------------------------------------
2+
3+
I tried updating the suppression file in the review/suppressed/ folder, but failed to write to review/suppressed/NoUnused.Variables.json.
4+
5+
Please check that elm-review has write permissions to that file and folder. In case it helps, here's the error I encountered:
6+
7+
Error: EACCES: permission denied, open '<local-path>/test/project-with-suppressed-errors-no-write/review/suppressed/NoUnused.Variables.json'
8+

‎test/suppress.test.js

+13-4
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,17 @@ test('Running with "suppress --check-after-tests" when there are uncommitted cha
5454
});
5555

5656
test('Running with unsupported version of suppression files should exit with failure', async () => {
57-
const output = await TestCli.runAndExpectError('', {
58-
project: 'project-with-unsupported-suppression-version'
59-
});
60-
expect(output).toMatchFile(testName('unsupported-suppression-version'));
57+
// In this setup, running `elm-review` should update a suppression file because
58+
// an unused variables issue has been fixed. It should however fail because
59+
// write permission has been removed from `review/suppressed/NoUnused.Variables.json`
60+
const project = 'project-with-suppressed-errors-no-write';
61+
const filePath = path.resolve(
62+
__dirname,
63+
project,
64+
'review/suppressed/NoUnused.Variables.json'
65+
);
66+
childProcess.execSync(`chmod -w ${filePath}`);
67+
68+
const output = await TestCli.runAndExpectError('', { project });
69+
expect(output).toMatchFile(testName('write-failure'));
6170
});

0 commit comments

Comments
 (0)