Skip to content

Commit a8641c3

Browse files
committed
Move rule output control logic to result package
Since output always has had the same format regardless of the tool configuration, with the configuration only determining whether or not it will be printed, it made sense to place the printing control logic at the point of printing, always outputting the message from the result package. It has now been determined that there are different formatting requirements for verbose and non-verbose output. This means that it makes more sense to move that logic to the result package and simply return an empty string when no output should be printed. This refactoring in preparation for the formatting changes to the non-verbose output.
1 parent 71feecf commit a8641c3

File tree

3 files changed

+22
-12
lines changed

3 files changed

+22
-12
lines changed

Diff for: internal/result/result.go

+8-5
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,6 @@ func (results *Type) Record(lintedProject project.Type, ruleConfiguration ruleco
101101
panic(fmt.Errorf("Error while determining rule level: %v", err))
102102
}
103103

104-
summaryText := fmt.Sprintf("Rule %s result: %s", ruleConfiguration.ID, ruleResult)
105-
106104
ruleMessage := ""
107105
if ruleResult == ruleresult.Fail {
108106
ruleMessage = message(ruleConfiguration.MessageTemplate, ruleOutput)
@@ -112,9 +110,14 @@ func (results *Type) Record(lintedProject project.Type, ruleConfiguration ruleco
112110
ruleMessage = ruleOutput
113111
}
114112

115-
// Add explanation of rule result if present.
116-
if ruleMessage != "" {
117-
summaryText += fmt.Sprintf("\n%s: %s", ruleLevel, ruleMessage)
113+
summaryText := ""
114+
if (ruleResult == ruleresult.Fail) || configuration.Verbose() {
115+
summaryText = fmt.Sprintf("Rule %s result: %s", ruleConfiguration.ID, ruleResult)
116+
// Add explanation of rule result if present.
117+
if ruleMessage != "" {
118+
summaryText += fmt.Sprintf("\n%s: %s", ruleLevel, ruleMessage)
119+
}
120+
summaryText += "\n"
118121
}
119122

120123
reportExists, projectReportIndex := results.getProjectReportIndex(lintedProject.Path)

Diff for: internal/result/result_test.go

+13-3
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,22 @@ func TestRecord(t *testing.T) {
7474
results.Initialize()
7575
ruleConfiguration := ruleconfiguration.Configurations()[0]
7676
ruleOutput := "foo"
77+
flags.Set("verbose", "true")
78+
require.Nil(t, configuration.Initialize(flags, projectPaths))
7779
summaryText := results.Record(lintedProject, ruleConfiguration, ruleresult.Fail, ruleOutput)
78-
assert.Equal(t, fmt.Sprintf("Rule %s result: %s\n%s: %s", ruleConfiguration.ID, ruleresult.Fail, rulelevel.Error, message(ruleConfiguration.MessageTemplate, ruleOutput)), summaryText)
80+
assert.Equal(t, fmt.Sprintf("Rule %s result: %s\n%s: %s\n", ruleConfiguration.ID, ruleresult.Fail, rulelevel.Error, message(ruleConfiguration.MessageTemplate, ruleOutput)), summaryText)
81+
summaryText = results.Record(lintedProject, ruleConfiguration, ruleresult.NotRun, ruleOutput)
82+
assert.Equal(t, fmt.Sprintf("Rule %s result: %s\n%s: %s\n", ruleConfiguration.ID, ruleresult.NotRun, rulelevel.Notice, ruleOutput), summaryText, "Non-fail result should not use message")
83+
summaryText = results.Record(lintedProject, ruleConfiguration, ruleresult.Pass, "")
84+
assert.Equal(t, fmt.Sprintf("Rule %s result: %s\n", ruleConfiguration.ID, ruleresult.Pass), summaryText, "Non-failure result with no rule function output should only use preface")
85+
flags.Set("verbose", "false")
86+
require.Nil(t, configuration.Initialize(flags, projectPaths))
87+
summaryText = results.Record(lintedProject, ruleConfiguration, ruleresult.Fail, ruleOutput)
88+
assert.Equal(t, fmt.Sprintf("Rule %s result: %s\n%s: %s\n", ruleConfiguration.ID, ruleresult.Fail, rulelevel.Error, message(ruleConfiguration.MessageTemplate, ruleOutput)), summaryText)
7989
summaryText = results.Record(lintedProject, ruleConfiguration, ruleresult.NotRun, ruleOutput)
80-
assert.Equal(t, fmt.Sprintf("Rule %s result: %s\n%s: %s", ruleConfiguration.ID, ruleresult.NotRun, rulelevel.Notice, ruleOutput), summaryText, "Non-fail result should not use message")
90+
assert.Equal(t, "", summaryText, "Non-fail result should not result in output in non-verbose mode")
8191
summaryText = results.Record(lintedProject, ruleConfiguration, ruleresult.Pass, "")
82-
assert.Equal(t, fmt.Sprintf("Rule %s result: %s", ruleConfiguration.ID, ruleresult.Pass), summaryText, "Non-failure result with no rule function output should only use preface")
92+
assert.Equal(t, "", summaryText, "Non-fail result should not result in output in non-verbose mode")
8393

8494
flags.Set("verbose", "true")
8595
require.Nil(t, configuration.Initialize(flags, projectPaths))

Diff for: internal/rule/rule.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626
"github.com/arduino/arduino-lint/internal/result"
2727
"github.com/arduino/arduino-lint/internal/result/feedback"
2828
"github.com/arduino/arduino-lint/internal/rule/ruleconfiguration"
29-
"github.com/arduino/arduino-lint/internal/rule/ruleresult"
3029
"github.com/sirupsen/logrus"
3130
)
3231

@@ -52,9 +51,7 @@ func Runner(project project.Type) {
5251

5352
ruleResult, ruleOutput := ruleConfiguration.RuleFunction()
5453
reportText := result.Results.Record(project, ruleConfiguration, ruleResult, ruleOutput)
55-
if (ruleResult == ruleresult.Fail) || configuration.Verbose() {
56-
feedback.Println(reportText)
57-
}
54+
feedback.Print(reportText)
5855
}
5956
}
6057

0 commit comments

Comments
 (0)