Skip to content

Commit a530769

Browse files
RKushnirRKushnir
authored
Detect and warn about incorrectly used ignore-comments (#325)
* Detect and warn about incorrectly used ignore-comments Resolves #197. * Keep existing ignoring behavior * Improve formatting * Test more ignore-related warnings * Remove warning in the case of ignore-next-line at the EOF * Test the warning output * Add a changelog entry * Adjust test descriptions --------- Co-authored-by: Roman <205906+RKushnir@users.noreply.github.com>
1 parent d09ee95 commit a530769

26 files changed

+365
-88
lines changed

‎CHANGELOG.md

+6
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
Unreleased
2+
------
3+
#### Enhancements
4+
- Print warnings about incorrectly used ignore-markers (#325), such as start-marker
5+
without a corresponding stop-marker, or two start-markers without a stop-marker in-between etc.
6+
17
0.18.1
28
------
39
#### Changes

‎lib/excoveralls/circle.ex

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ defmodule ExCoveralls.Circle do
33
Handles circle-ci integration with coveralls.
44
"""
55
alias ExCoveralls.Poster
6+
alias ExCoveralls.Stats
67

78
def execute(stats, options) do
89
json = generate_json(stats, Enum.into(options, %{}))
@@ -20,7 +21,7 @@ defmodule ExCoveralls.Circle do
2021
service_number: get_number(),
2122
service_job_id: get_job_id(),
2223
service_pull_request: get_pull_request(),
23-
source_files: stats,
24+
source_files: Stats.serialize(stats),
2425
git: generate_git_info(),
2526
parallel: options[:parallel],
2627
flag_name: options[:flagname]

‎lib/excoveralls/drone.ex

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ defmodule ExCoveralls.Drone do
33
Handles drone-ci integration with coveralls.
44
"""
55
alias ExCoveralls.Poster
6+
alias ExCoveralls.Stats
67

78
def execute(stats, options) do
89
json = generate_json(stats, Enum.into(options, %{}))
@@ -20,7 +21,7 @@ defmodule ExCoveralls.Drone do
2021
service_number: get_build_num(),
2122
service_job_id: get_build_num(),
2223
service_pull_request: get_pull_request(),
23-
source_files: stats,
24+
source_files: Stats.serialize(stats),
2425
git: generate_git_info(),
2526
parallel: options[:parallel],
2627
flag_name: options[:flagname]

‎lib/excoveralls/github.ex

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ defmodule ExCoveralls.Github do
33
Handles GitHub Actions integration with coveralls.
44
"""
55
alias ExCoveralls.Poster
6+
alias ExCoveralls.Stats
67

78
def execute(stats, options) do
89
json = generate_json(stats, Enum.into(options, %{}))
@@ -20,7 +21,7 @@ defmodule ExCoveralls.Github do
2021
%{
2122
repo_token: get_env("GITHUB_TOKEN"),
2223
service_name: "github",
23-
source_files: stats,
24+
source_files: Stats.serialize(stats),
2425
parallel: options[:parallel],
2526
flag_name: options[:flagname],
2627
git: git_info()

‎lib/excoveralls/gitlab.ex

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ defmodule ExCoveralls.Gitlab do
33
Handles gitlab-ci integration with coveralls.
44
"""
55
alias ExCoveralls.Poster
6+
alias ExCoveralls.Stats
67

78
def execute(stats, options) do
89
json = generate_json(stats, Enum.into(options, %{}))
@@ -23,7 +24,7 @@ defmodule ExCoveralls.Gitlab do
2324
service_number: get_number(),
2425
service_job_id: get_job_id(),
2526
service_pull_request: get_pull_request(),
26-
source_files: stats,
27+
source_files: Stats.serialize(stats),
2728
git: generate_git_info(),
2829
parallel: options[:parallel],
2930
flag_name: options[:flagname]

‎lib/excoveralls/ignore.ex

+151-35
Original file line numberDiff line numberDiff line change
@@ -10,51 +10,167 @@ defmodule ExCoveralls.Ignore do
1010
Enum.map(info, &do_filter/1)
1111
end
1212

13+
defmodule State do
14+
defstruct ignore_mode: :no_ignore,
15+
coverage: [],
16+
coverage_buffer: [],
17+
warnings: [],
18+
last_marker_index: nil
19+
end
20+
1321
defp do_filter(%{name: name, source: source, coverage: coverage}) do
14-
lines = String.split(source, "\n")
15-
list = Enum.zip(lines, coverage)
16-
|> Enum.map_reduce(:no_ignore, &check_and_swap/2)
17-
|> elem(0)
18-
|> List.zip
19-
|> Enum.map(&Tuple.to_list(&1))
22+
source_lines = String.split(source, "\n")
2023

21-
[source, coverage] = parse_filter_list(list)
22-
%{name: name, source: source, coverage: coverage}
23-
end
24+
processing_result =
25+
Enum.zip(source_lines, coverage)
26+
|> Enum.with_index()
27+
|> Enum.reduce(%State{}, &process_line/2)
28+
|> process_end_of_file()
2429

25-
defp check_and_swap({line, coverage}, ignore) do
26-
{
27-
coverage_for_line({line, coverage}, ignore),
28-
ignore_next?(line, ignore)
29-
}
30+
updated_coverage = processing_result.coverage |> List.flatten() |> Enum.reverse()
31+
warnings = Enum.sort_by(processing_result.warnings, &elem(&1, 0))
32+
%{name: name, source: source, coverage: updated_coverage, warnings: warnings}
3033
end
3134

32-
defp parse_filter_list([]), do: ["", []]
33-
defp parse_filter_list([lines, coverage]), do: [Enum.join(lines, "\n"), coverage]
34-
35-
defp coverage_for_line({line, coverage}, ignore) do
36-
if ignore == :no_ignore do
37-
{line, coverage}
38-
else
39-
{line, nil}
35+
defp process_line({{source_line, coverage_line}, index}, state) do
36+
case detect_ignore_marker(source_line) do
37+
:none -> process_regular_line(coverage_line, index, state)
38+
:start -> process_start_marker(coverage_line, index, state)
39+
:stop -> process_stop_marker(coverage_line, index, state)
40+
:next_line -> process_next_line_marker(coverage_line, index, state)
4041
end
4142
end
4243

43-
defp ignore_next?(line, ignore) do
44+
defp detect_ignore_marker(line) do
4445
case Regex.run(~r/coveralls-ignore-(start|stop|next-line)/, line, capture: :all_but_first) do
45-
["start"] -> :ignore_block
46-
["stop"] -> :no_ignore
47-
["next-line"] ->
48-
case ignore do
49-
:ignore_block -> ignore
50-
_sth -> :ignore_line
51-
end
52-
_sth ->
53-
case ignore do
54-
:ignore_line -> :no_ignore
55-
_sth -> ignore
56-
end
46+
["start"] -> :start
47+
["stop"] -> :stop
48+
["next-line"] -> :next_line
49+
_sth -> :none
5750
end
5851
end
5952

53+
defp process_regular_line(
54+
coverage_line,
55+
_index,
56+
state = %{ignore_mode: :no_ignore, coverage_buffer: []}
57+
) do
58+
%{state | coverage: [coverage_line | state.coverage]}
59+
end
60+
61+
defp process_regular_line(_coverage_line, _index, state = %{ignore_mode: :ignore_line}) do
62+
%{state | ignore_mode: :no_ignore, coverage: [nil | state.coverage]}
63+
end
64+
65+
defp process_regular_line(_coverage_line, _index, state = %{ignore_mode: :ignore_block}) do
66+
%{state | coverage: [nil | state.coverage]}
67+
end
68+
69+
defp process_start_marker(
70+
_coverage_line,
71+
index,
72+
state = %{ignore_mode: :no_ignore}
73+
) do
74+
%{
75+
state
76+
| ignore_mode: :ignore_block,
77+
coverage: [nil | state.coverage],
78+
last_marker_index: index
79+
}
80+
end
81+
82+
defp process_start_marker(_coverage_line, index, state = %{ignore_mode: :ignore_block}) do
83+
warning = {index, "unexpected ignore-start or missing previous ignore-stop"}
84+
85+
%{
86+
state
87+
| coverage: [nil | state.coverage],
88+
warnings: [warning | state.warnings],
89+
last_marker_index: index
90+
}
91+
end
92+
93+
defp process_start_marker(_coverage_line, index, state = %{ignore_mode: :ignore_line}) do
94+
warning = {state.last_marker_index, "redundant ignore-next-line right before an ignore-start"}
95+
96+
%{
97+
state
98+
| ignore_mode: :ignore_block,
99+
coverage: [nil | state.coverage],
100+
warnings: [warning | state.warnings],
101+
last_marker_index: index
102+
}
103+
end
104+
105+
defp process_stop_marker(_coverage_line, index, state = %{ignore_mode: :ignore_block}) do
106+
%{
107+
state
108+
| ignore_mode: :no_ignore,
109+
coverage: [nil | state.coverage],
110+
last_marker_index: index
111+
}
112+
end
113+
114+
defp process_stop_marker(_coverage_line, index, state) do
115+
warning = {index, "unexpected ignore-stop or missing previous ignore-start"}
116+
117+
%{
118+
state
119+
| ignore_mode: :no_ignore,
120+
coverage: [nil | state.coverage],
121+
warnings: [warning | state.warnings],
122+
last_marker_index: index
123+
}
124+
end
125+
126+
defp process_next_line_marker(
127+
_coverage_line,
128+
index,
129+
state = %{ignore_mode: :no_ignore}
130+
) do
131+
%{
132+
state
133+
| ignore_mode: :ignore_line,
134+
coverage: [nil | state.coverage],
135+
last_marker_index: index
136+
}
137+
end
138+
139+
defp process_next_line_marker(
140+
_coverage_line,
141+
index,
142+
state = %{ignore_mode: :ignore_block}
143+
) do
144+
warning = {index, "redundant ignore-next-line inside ignore block"}
145+
146+
%{
147+
state
148+
| coverage: [nil | state.coverage],
149+
warnings: [warning | state.warnings]
150+
}
151+
end
152+
153+
defp process_next_line_marker(
154+
_coverage_line,
155+
index,
156+
state = %{ignore_mode: :ignore_line}
157+
) do
158+
warning = {index, "duplicated ignore-next-line"}
159+
160+
%{
161+
state
162+
| coverage: [nil | state.coverage],
163+
warnings: [warning | state.warnings],
164+
last_marker_index: index
165+
}
166+
end
167+
168+
defp process_end_of_file(state = %{ignore_mode: :ignore_block}) do
169+
warning =
170+
{state.last_marker_index, "ignore-start without a corresponding ignore-stop"}
171+
172+
%{state | warnings: [warning | state.warnings]}
173+
end
174+
175+
defp process_end_of_file(state), do: state
60176
end

‎lib/excoveralls/json.ex

+2-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ defmodule ExCoveralls.Json do
22
@moduledoc """
33
Generate JSON output for results.
44
"""
5+
alias ExCoveralls.Stats
56

67
@file_name "excoveralls.json"
78

@@ -16,7 +17,7 @@ defmodule ExCoveralls.Json do
1617

1718
def generate_json(stats, _options) do
1819
Jason.encode!(%{
19-
source_files: stats
20+
source_files: Stats.serialize(stats)
2021
})
2122
end
2223

‎lib/excoveralls/local.ex

+7
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ defmodule ExCoveralls.Local do
4646
enabled = ExCoveralls.Settings.get_print_summary
4747
if enabled and not ExCoveralls.ConfServer.summary_printed?() do
4848
coverage(stats, options) |> IO.puts()
49+
warnings(stats) |> IO.write()
4950
ExCoveralls.ConfServer.summary_printed()
5051
end
5152
end
@@ -92,6 +93,12 @@ defmodule ExCoveralls.Local do
9293
end
9394
end
9495

96+
def warnings(stats) do
97+
for stat <- stats, {line_num, message} <- stat[:warnings], into: "" do
98+
print_string("\e[33mwarning:\e[m ~s\n ~s:~b\n", [message, stat[:name], line_num + 1])
99+
end
100+
end
101+
95102
defp sort(count_info, options) do
96103
if options[:sort] do
97104
sort_order = parse_sort_options(options)

‎lib/excoveralls/post.ex

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ defmodule ExCoveralls.Post do
33
Handles general-purpose CI integration with coveralls.
44
"""
55
alias ExCoveralls.Poster
6+
alias ExCoveralls.Stats
67

78
def execute(stats, options) do
89
json = generate_json(stats, options)
@@ -17,7 +18,7 @@ defmodule ExCoveralls.Post do
1718
repo_token: options[:token],
1819
service_name: options[:service_name],
1920
service_number: options[:service_number],
20-
source_files: source_info,
21+
source_files: Stats.serialize(source_info),
2122
parallel: options[:parallel],
2223
flag_name: options[:flagname],
2324
git: generate_git_info(options)

‎lib/excoveralls/semaphore.ex

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ defmodule ExCoveralls.Semaphore do
33
Handles semaphore-ci integration with coveralls.
44
"""
55
alias ExCoveralls.Poster
6+
alias ExCoveralls.Stats
67

78
def execute(stats, options) do
89
json = generate_json(stats, Enum.into(options, %{}))
@@ -20,7 +21,7 @@ defmodule ExCoveralls.Semaphore do
2021
service_number: get_build_num(),
2122
service_job_id: get_build_num(),
2223
service_pull_request: get_pull_request(),
23-
source_files: stats,
24+
source_files: Stats.serialize(stats),
2425
git: generate_git_info(),
2526
parallel: options[:parallel],
2627
flag_name: options[:flagname]

‎lib/excoveralls/stats.ex

+8
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,14 @@ defmodule ExCoveralls.Stats do
223223
%Line{coverage: Enum.at(coverage, i) , source: line}
224224
end
225225

226+
@doc """
227+
Converts coverage stats to a map, which can be serialized to JSON
228+
for posting it to Coveralls or writing to excoveralls.json.
229+
"""
230+
def serialize(stats) do
231+
Enum.map(stats, &Map.take(&1, [:name, :source, :coverage]))
232+
end
233+
226234
@doc """
227235
Exit the process with a status of 1 if coverage is below the minimum.
228236
"""

‎lib/excoveralls/travis.ex

+3-2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ defmodule ExCoveralls.Travis do
33
Handles travis-ci integration with coveralls.
44
"""
55
alias ExCoveralls.Poster
6+
alias ExCoveralls.Stats
67

78
def execute(stats, options) do
89
json = generate_json(stats, Enum.into(options, %{}))
@@ -18,15 +19,15 @@ defmodule ExCoveralls.Travis do
1819
service_job_id: get_job_id(),
1920
service_name: "travis-pro",
2021
repo_token: get_repo_token(),
21-
source_files: stats,
22+
source_files: Stats.serialize(stats),
2223
git: generate_git_info()
2324
})
2425
end
2526
def generate_json(stats, _options) do
2627
Jason.encode!(%{
2728
service_job_id: get_job_id(),
2829
service_name: "travis-ci",
29-
source_files: stats,
30+
source_files: Stats.serialize(stats),
3031
git: generate_git_info()
3132
})
3233
end

0 commit comments

Comments
 (0)