Skip to content

Commit 035fcf7

Browse files
committed
Change --permissive flag to --compliance
The initial design of the tool was based on the idea of simply enforcing the Arduino project specifications. It soon became clear that, due to the legacy of not strictly enforcing specification compliance on Library Manager libraries where the non-compliance doesn't result in a significant problem, there was a need for a "permissive" mode. In addition, the need was found for a "strict" mode, in which the tool will advocate best practices above and beyond the specifications. For this purpose, the `--permissive` flag was added. However, along the way the original concept of enforcing the specifications was lost. Permissive mode is an unfortunate necessity. Strict mode is great, but may not be to some people's taste. So a configuration mode where the tool simply check specification compliance remains useful, and is a reasonable default.
1 parent 5f24a05 commit 035fcf7

File tree

7 files changed

+76
-36
lines changed

7 files changed

+76
-36
lines changed

Diff for: check/check_test.go

+10-10
Original file line numberDiff line numberDiff line change
@@ -35,25 +35,25 @@ func Test_shouldRun(t *testing.T) {
3535
disableModes []checkmode.Type
3636
enableModes []checkmode.Type
3737
libraryManagerSetting string
38-
permissiveSetting string
38+
complianceSetting string
3939
shouldRunAssertion assert.BoolAssertionFunc
4040
errorAssertion assert.ErrorAssertionFunc
4141
}{
42-
{"Project type mismatch", projecttype.Library, projecttype.Sketch, []checkmode.Type{}, []checkmode.Type{}, "false", "false", assert.False, assert.NoError},
43-
{"Disable mode match", projecttype.Library, projecttype.Library, []checkmode.Type{checkmode.LibraryManagerSubmission}, []checkmode.Type{}, "submit", "false", assert.False, assert.NoError},
44-
{"Enable mode match", projecttype.Library, projecttype.Library, []checkmode.Type{}, []checkmode.Type{checkmode.LibraryManagerSubmission}, "submit", "false", assert.True, assert.NoError},
45-
{"Disable mode default", projecttype.Library, projecttype.Library, []checkmode.Type{checkmode.Default}, []checkmode.Type{checkmode.LibraryManagerSubmission}, "update", "false", assert.False, assert.NoError},
46-
{"Disable mode default override", projecttype.Library, projecttype.Library, []checkmode.Type{checkmode.Default}, []checkmode.Type{checkmode.LibraryManagerSubmission}, "submit", "false", assert.True, assert.NoError},
47-
{"Enable mode default", projecttype.Library, projecttype.Library, []checkmode.Type{checkmode.LibraryManagerSubmission}, []checkmode.Type{checkmode.Default}, "update", "false", assert.True, assert.NoError},
48-
{"Disable mode default override", projecttype.Library, projecttype.Library, []checkmode.Type{checkmode.LibraryManagerSubmission}, []checkmode.Type{checkmode.Default}, "submit", "false", assert.False, assert.NoError},
49-
{"Unable to resolve", projecttype.Library, projecttype.Library, []checkmode.Type{checkmode.LibraryManagerSubmission}, []checkmode.Type{checkmode.LibraryManagerIndexed}, "false", "false", assert.False, assert.Error},
42+
{"Project type mismatch", projecttype.Library, projecttype.Sketch, []checkmode.Type{}, []checkmode.Type{}, "false", "specification", assert.False, assert.NoError},
43+
{"Disable mode match", projecttype.Library, projecttype.Library, []checkmode.Type{checkmode.LibraryManagerSubmission}, []checkmode.Type{}, "submit", "specification", assert.False, assert.NoError},
44+
{"Enable mode match", projecttype.Library, projecttype.Library, []checkmode.Type{}, []checkmode.Type{checkmode.LibraryManagerSubmission}, "submit", "specification", assert.True, assert.NoError},
45+
{"Disable mode default", projecttype.Library, projecttype.Library, []checkmode.Type{checkmode.Default}, []checkmode.Type{checkmode.LibraryManagerSubmission}, "update", "specification", assert.False, assert.NoError},
46+
{"Disable mode default override", projecttype.Library, projecttype.Library, []checkmode.Type{checkmode.Default}, []checkmode.Type{checkmode.LibraryManagerSubmission}, "submit", "specification", assert.True, assert.NoError},
47+
{"Enable mode default", projecttype.Library, projecttype.Library, []checkmode.Type{checkmode.LibraryManagerSubmission}, []checkmode.Type{checkmode.Default}, "update", "specification", assert.True, assert.NoError},
48+
{"Disable mode default override", projecttype.Library, projecttype.Library, []checkmode.Type{checkmode.LibraryManagerSubmission}, []checkmode.Type{checkmode.Default}, "submit", "specification", assert.False, assert.NoError},
49+
{"Unable to resolve", projecttype.Library, projecttype.Library, []checkmode.Type{checkmode.LibraryManagerSubmission}, []checkmode.Type{checkmode.LibraryManagerIndexed}, "false", "specification", assert.False, assert.Error},
5050
}
5151

5252
flags := test.ConfigurationFlags()
5353

5454
for _, testTable := range testTables {
5555
flags.Set("library-manager", testTable.libraryManagerSetting)
56-
flags.Set("permissive", testTable.permissiveSetting)
56+
flags.Set("compliance", testTable.complianceSetting)
5757

5858
configuration.Initialize(flags, []string{"/foo"})
5959

Diff for: cli/cli.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ func Root() *cobra.Command {
3131
Run: command.ArduinoCheck,
3232
}
3333

34+
rootCommand.PersistentFlags().String("compliance", "specification", "Configure how strict the tool is. Can be {strict|specification|permissive}")
3435
rootCommand.PersistentFlags().String("format", "text", "The output format can be {text|json}.")
3536
rootCommand.PersistentFlags().String("library-manager", "", "Configure the checks for libraries in the Arduino Library Manager index. Can be {submit|update|false}.\nsubmit: Also run additional checks required to pass before a library is accepted for inclusion in the index.\nupdate: Also run additional checks required to pass before new releases of a library already in the index are accepted.\nfalse: Don't run any Library Manager-specific checks.")
3637
rootCommand.PersistentFlags().String("log-format", "text", "The output format for the logs, can be {text|json}.")
3738
rootCommand.PersistentFlags().String("log-level", "panic", "Messages with this level and above will be logged. Valid levels are: trace, debug, info, warn, error, fatal, panic")
38-
rootCommand.PersistentFlags().Bool("permissive", false, "Only fail when critical issues are detected.")
3939
rootCommand.PersistentFlags().String("project-type", "all", "Only check projects of the specified type and their subprojects. Can be {sketch|library|all}.")
4040
rootCommand.PersistentFlags().Bool("recursive", true, "Search path recursively for Arduino projects to check. Can be {true|false}.")
4141
rootCommand.PersistentFlags().String("report-file", "", "Save a report on the checks to this file.")

Diff for: configuration/checkmode/checkmode.go

+17-1
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,30 @@ type Type int
2929

3030
//go:generate stringer -type=Type -linecomment
3131
const (
32-
Permissive Type = iota // --permissive
32+
Strict Type = iota // strict
33+
Specification // specification
34+
Permissive // permissive
3335
LibraryManagerSubmission // --library-manager=submit
3436
LibraryManagerIndexed // --library-manager=update
3537
Official // ARDUINO_CHECK_OFFICIAL
3638
All // always
3739
Default // default
3840
)
3941

42+
// ComplianceModeFromString parses the --compliance flag value and returns the corresponding check mode settings.
43+
func ComplianceModeFromString(complianceModeString string) (bool, bool, bool, error) {
44+
switch strings.ToLower(complianceModeString) {
45+
case "strict":
46+
return true, false, false, nil
47+
case "specification":
48+
return false, true, false, nil
49+
case "permissive":
50+
return false, false, true, nil
51+
default:
52+
return false, false, false, fmt.Errorf("No matching compliance mode for string %s", complianceModeString)
53+
}
54+
}
55+
4056
// LibraryManagerModeFromString parses the --library-manager flag value and returns the corresponding check mode settings.
4157
func LibraryManagerModeFromString(libraryManagerModeString string) (bool, bool, error) {
4258
switch strings.ToLower(libraryManagerModeString) {

Diff for: configuration/checkmode/type_string.go

+10-8
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: configuration/configuration.go

+12-3
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,15 @@ import (
3333
// Initialize sets up the tool configuration according to defaults and user-specified options.
3434
func Initialize(flags *pflag.FlagSet, projectPaths []string) error {
3535
var err error
36+
37+
complianceString, _ := flags.GetString("compliance")
38+
if complianceString != "" {
39+
customCheckModes[checkmode.Strict], customCheckModes[checkmode.Specification], customCheckModes[checkmode.Permissive], err = checkmode.ComplianceModeFromString(complianceString)
40+
if err != nil {
41+
return fmt.Errorf("--compliance flag value %s not valid", complianceString)
42+
}
43+
}
44+
3645
outputFormatString, _ := flags.GetString("format")
3746
outputFormat, err = outputformat.FromString(outputFormatString)
3847
if err != nil {
@@ -61,8 +70,6 @@ func Initialize(flags *pflag.FlagSet, projectPaths []string) error {
6170
}
6271
logrus.SetLevel(logLevel)
6372

64-
customCheckModes[checkmode.Permissive], _ = flags.GetBool("permissive")
65-
6673
superprojectTypeFilterString, _ := flags.GetString("project-type")
6774
superprojectTypeFilter, err = projecttype.FromString(superprojectTypeFilterString)
6875
if err != nil {
@@ -103,12 +110,14 @@ func Initialize(flags *pflag.FlagSet, projectPaths []string) error {
103110
}
104111

105112
logrus.WithFields(logrus.Fields{
113+
"compliance strict mode": customCheckModes[checkmode.Strict],
114+
"compliance specification mode": customCheckModes[checkmode.Specification],
115+
"compliance permissive mode": customCheckModes[checkmode.Permissive],
106116
"output format": OutputFormat(),
107117
"Library Manager submission mode": customCheckModes[checkmode.LibraryManagerSubmission],
108118
"Library Manager update mode": customCheckModes[checkmode.LibraryManagerIndexed],
109119
"log format": logFormatString,
110120
"log level": logrus.GetLevel().String(),
111-
"permissive": customCheckModes[checkmode.Permissive],
112121
"superproject type filter": SuperprojectTypeFilter(),
113122
"recursive": Recursive(),
114123
"report file": ReportFilePath(),

Diff for: configuration/configuration_test.go

+25-12
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,31 @@ func init() {
3838
projectPaths = []string{projectPath}
3939
}
4040

41+
func TestInitializeCompliance(t *testing.T) {
42+
flags := test.ConfigurationFlags()
43+
44+
flags.Set("compliance", "foo")
45+
assert.Error(t, Initialize(flags, projectPaths))
46+
47+
flags.Set("compliance", "strict")
48+
assert.Nil(t, Initialize(flags, projectPaths))
49+
assert.True(t, customCheckModes[checkmode.Strict])
50+
assert.False(t, customCheckModes[checkmode.Specification])
51+
assert.False(t, customCheckModes[checkmode.Permissive])
52+
53+
flags.Set("compliance", "specification")
54+
assert.Nil(t, Initialize(flags, projectPaths))
55+
assert.False(t, customCheckModes[checkmode.Strict])
56+
assert.True(t, customCheckModes[checkmode.Specification])
57+
assert.False(t, customCheckModes[checkmode.Permissive])
58+
59+
flags.Set("compliance", "permissive")
60+
assert.Nil(t, Initialize(flags, projectPaths))
61+
assert.False(t, customCheckModes[checkmode.Strict])
62+
assert.False(t, customCheckModes[checkmode.Specification])
63+
assert.True(t, customCheckModes[checkmode.Permissive])
64+
}
65+
4166
func TestInitializeFormat(t *testing.T) {
4267
flags := test.ConfigurationFlags()
4368
flags.Set("format", "foo")
@@ -94,18 +119,6 @@ func TestInitializeLogFormat(t *testing.T) {
94119
assert.Nil(t, Initialize(flags, projectPaths))
95120
}
96121

97-
func TestInitializePermissive(t *testing.T) {
98-
flags := test.ConfigurationFlags()
99-
100-
flags.Set("permissive", "true")
101-
assert.Nil(t, Initialize(flags, projectPaths))
102-
assert.True(t, customCheckModes[checkmode.Permissive])
103-
104-
flags.Set("permissive", "false")
105-
assert.Nil(t, Initialize(flags, projectPaths))
106-
assert.False(t, customCheckModes[checkmode.Permissive])
107-
}
108-
109122
func TestInitializeProjectType(t *testing.T) {
110123
flags := test.ConfigurationFlags()
111124

Diff for: util/test/test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ import "github.com/spf13/pflag"
2121
// ConfigurationFlags returns a set of the flags used for command line configuration of arduino-check.
2222
func ConfigurationFlags() *pflag.FlagSet {
2323
flags := pflag.NewFlagSet("", pflag.ExitOnError)
24+
flags.String("compliance", "specification", "")
2425
flags.String("format", "text", "")
2526
flags.String("library-manager", "", "")
2627
flags.String("log-format", "text", "")
2728
flags.String("log-level", "panic", "")
28-
flags.Bool("permissive", false, "")
2929
flags.String("project-type", "all", "")
3030
flags.Bool("recursive", true, "")
3131
flags.String("report-file", "", "")

0 commit comments

Comments
 (0)