Skip to content

Commit 090caf8

Browse files
per1234per1234
committed
Use appropriate check result value when determined not relevant
There are various extents to which a check can be run: 1. Skipped due to check configuration (e.g., check is configured to run for libraries but the project is a sketch). 2. Not run due to a problem not directly related to the check (e.g., invalid library.properties data format). 3. Skipped because the check function determined it was not applicable (e.g., formating check on undefined optional library.properties field). 4. Run. Each has an associated check result value: Skip, NotRun, Pass/Fail. Previously, the "NotRun" result was used for both (2) and (3), but "Skip" is much more appropriate for (3) because this is a situation where there was no reason to run the check, whereas "NotRun" indicates that the check should have been run, but couldn't.
1 parent dab9db8 commit 090caf8

File tree

6 files changed

+99
-80
lines changed

6 files changed

+99
-80
lines changed

Diff for: ‎check/checkfunctions/checkfunctions.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func containsIncorrectPathBaseCase(pathList paths.PathList, correctBaseName stri
7272
// MissingReadme checks if the project has a readme that will be recognized by GitHub.
7373
func MissingReadme() (result checkresult.Type, output string) {
7474
if checkdata.ProjectType() != checkdata.SuperProjectType() {
75-
return checkresult.NotRun, "Readme not required for subprojects"
75+
return checkresult.Skip, "Readme not required for subprojects"
7676
}
7777

7878
// https://github.com/github/markup/blob/master/README.md
@@ -91,7 +91,7 @@ func MissingReadme() (result checkresult.Type, output string) {
9191
// MissingLicenseFile checks if the project has a license file that will be recognized by GitHub.
9292
func MissingLicenseFile() (result checkresult.Type, output string) {
9393
if checkdata.ProjectType() != checkdata.SuperProjectType() {
94-
return checkresult.NotRun, "License file not required for subprojects"
94+
return checkresult.Skip, "License file not required for subprojects"
9595
}
9696

9797
// https://docs.github.com/en/free-pro-team@latest/github/creating-cloning-and-archiving-repositories/licensing-a-repository#detecting-a-license

Diff for: ‎check/checkfunctions/checkfunctions_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func checkCheckFunction(checkFunction Type, testTables []checkFunctionTestTable,
6464

6565
func TestMissingReadme(t *testing.T) {
6666
testTables := []checkFunctionTestTable{
67-
{"Subproject", "readme", projecttype.Sketch, projecttype.Library, checkresult.NotRun, ""},
67+
{"Subproject", "readme", projecttype.Sketch, projecttype.Library, checkresult.Skip, ""},
6868
{"Readme", "readme", projecttype.Sketch, projecttype.Sketch, checkresult.Pass, ""},
6969
{"No readme", "no-readme", projecttype.Sketch, projecttype.Sketch, checkresult.Fail, ""},
7070
}
@@ -74,7 +74,7 @@ func TestMissingReadme(t *testing.T) {
7474

7575
func TestMissingLicenseFile(t *testing.T) {
7676
testTables := []checkFunctionTestTable{
77-
{"Subproject", "no-license-file", projecttype.Sketch, projecttype.Library, checkresult.NotRun, ""},
77+
{"Subproject", "no-license-file", projecttype.Sketch, projecttype.Library, checkresult.Skip, ""},
7878
{"Has license", "license-file", projecttype.Sketch, projecttype.Sketch, checkresult.Pass, ""},
7979
{"Has license in subfolder", "license-file-in-subfolder", projecttype.Sketch, projecttype.Sketch, checkresult.Fail, ""},
8080
{"No license", "no-license-file", projecttype.Sketch, projecttype.Sketch, checkresult.Fail, ""},

Diff for: ‎check/checkfunctions/library.go

+78-59
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import (
4343
// LibraryPropertiesFormat checks for invalid library.properties format.
4444
func LibraryPropertiesFormat() (result checkresult.Type, output string) {
4545
if checkdata.LoadedLibrary() != nil && checkdata.LoadedLibrary().IsLegacy {
46-
return checkresult.NotRun, "Library has no library.properties"
46+
return checkresult.Skip, "Library has no library.properties"
4747
}
4848

4949
if checkdata.LibraryPropertiesLoadError() != nil {
@@ -110,9 +110,12 @@ func RedundantLibraryProperties() (result checkresult.Type, output string) {
110110

111111
// LibraryPropertiesNameFieldMissing checks for missing library.properties "name" field.
112112
func LibraryPropertiesNameFieldMissing() (result checkresult.Type, output string) {
113-
shouldRun, reason := runRequiredLibraryPropertiesFieldCheck()
114-
if !shouldRun {
115-
return checkresult.NotRun, reason
113+
if checkdata.LibraryPropertiesLoadError() != nil {
114+
return checkresult.NotRun, "Couldn't load library.properties"
115+
}
116+
117+
if checkdata.LoadedLibrary().IsLegacy {
118+
return checkresult.Skip, "Library has legacy format"
116119
}
117120

118121
if schema.RequiredPropertyMissing("name", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -340,9 +343,12 @@ func LibraryPropertiesNameFieldHeaderMismatch() (result checkresult.Type, output
340343

341344
// LibraryPropertiesVersionFieldMissing checks for missing library.properties "version" field.
342345
func LibraryPropertiesVersionFieldMissing() (result checkresult.Type, output string) {
343-
shouldRun, reason := runRequiredLibraryPropertiesFieldCheck()
344-
if !shouldRun {
345-
return checkresult.NotRun, reason
346+
if checkdata.LibraryPropertiesLoadError() != nil {
347+
return checkresult.NotRun, "Couldn't load library.properties"
348+
}
349+
350+
if checkdata.LoadedLibrary().IsLegacy {
351+
return checkresult.Skip, "Library has legacy format"
346352
}
347353

348354
if schema.RequiredPropertyMissing("version", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -390,7 +396,7 @@ func LibraryPropertiesVersionFieldNonSemver() (result checkresult.Type, output s
390396
// LibraryPropertiesVersionFieldBehindTag checks whether a release tag was made without first bumping the library.properties version value.
391397
func LibraryPropertiesVersionFieldBehindTag() (result checkresult.Type, output string) {
392398
if checkdata.ProjectType() != checkdata.SuperProjectType() {
393-
return checkresult.NotRun, "Not relevant for subprojects"
399+
return checkresult.Skip, "Not relevant for subprojects"
394400
}
395401

396402
if checkdata.LibraryPropertiesLoadError() != nil {
@@ -410,7 +416,7 @@ func LibraryPropertiesVersionFieldBehindTag() (result checkresult.Type, output s
410416

411417
repository, err := git.PlainOpen(checkdata.ProjectPath().String())
412418
if err != nil {
413-
return checkresult.NotRun, "Project path is not a repository"
419+
return checkresult.Skip, "Project path is not a repository"
414420
}
415421

416422
headRef, err := repository.Head()
@@ -482,9 +488,12 @@ func LibraryPropertiesVersionFieldBehindTag() (result checkresult.Type, output s
482488

483489
// LibraryPropertiesAuthorFieldMissing checks for missing library.properties "author" field.
484490
func LibraryPropertiesAuthorFieldMissing() (result checkresult.Type, output string) {
485-
shouldRun, reason := runRequiredLibraryPropertiesFieldCheck()
486-
if !shouldRun {
487-
return checkresult.NotRun, reason
491+
if checkdata.LibraryPropertiesLoadError() != nil {
492+
return checkresult.NotRun, "Couldn't load library.properties"
493+
}
494+
495+
if checkdata.LoadedLibrary().IsLegacy {
496+
return checkresult.Skip, "Library has legacy format"
488497
}
489498

490499
if schema.RequiredPropertyMissing("author", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -512,9 +521,12 @@ func LibraryPropertiesAuthorFieldLTMinLength() (result checkresult.Type, output
512521

513522
// LibraryPropertiesMaintainerFieldMissing checks for missing library.properties "maintainer" field.
514523
func LibraryPropertiesMaintainerFieldMissing() (result checkresult.Type, output string) {
515-
shouldRun, reason := runRequiredLibraryPropertiesFieldCheck()
516-
if !shouldRun {
517-
return checkresult.NotRun, reason
524+
if checkdata.LibraryPropertiesLoadError() != nil {
525+
return checkresult.NotRun, "Couldn't load library.properties"
526+
}
527+
528+
if checkdata.LoadedLibrary().IsLegacy {
529+
return checkresult.Skip, "Library has legacy format"
518530
}
519531

520532
if schema.RequiredPropertyMissing("maintainer", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -565,7 +577,7 @@ func LibraryPropertiesEmailFieldAsMaintainerAlias() (result checkresult.Type, ou
565577
}
566578

567579
if !checkdata.LibraryProperties().ContainsKey("email") {
568-
return checkresult.NotRun, "Field not present"
580+
return checkresult.Skip, "Field not present"
569581
}
570582

571583
if !checkdata.LibraryProperties().ContainsKey("maintainer") {
@@ -582,7 +594,7 @@ func LibraryPropertiesEmailFieldLTMinLength() (result checkresult.Type, output s
582594
}
583595

584596
if checkdata.LibraryProperties().ContainsKey("maintainer") || !checkdata.LibraryProperties().ContainsKey("email") {
585-
return checkresult.NotRun, "Field not present"
597+
return checkresult.Skip, "Field not present"
586598
}
587599

588600
if schema.PropertyLessThanMinLength("email", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -604,7 +616,7 @@ func LibraryPropertiesEmailFieldStartsWithArduino() (result checkresult.Type, ou
604616

605617
email, ok := checkdata.LibraryProperties().GetOk("email")
606618
if !ok {
607-
return checkresult.NotRun, "Field not present"
619+
return checkresult.Skip, "Field not present"
608620
}
609621

610622
if schema.ValidationErrorMatch("^#/email$", "/patternObjects/notStartsWithArduino", "", "", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -616,9 +628,12 @@ func LibraryPropertiesEmailFieldStartsWithArduino() (result checkresult.Type, ou
616628

617629
// LibraryPropertiesSentenceFieldMissing checks for missing library.properties "sentence" field.
618630
func LibraryPropertiesSentenceFieldMissing() (result checkresult.Type, output string) {
619-
shouldRun, reason := runRequiredLibraryPropertiesFieldCheck()
620-
if !shouldRun {
621-
return checkresult.NotRun, reason
631+
if checkdata.LibraryPropertiesLoadError() != nil {
632+
return checkresult.NotRun, "Couldn't load library.properties"
633+
}
634+
635+
if checkdata.LoadedLibrary().IsLegacy {
636+
return checkresult.Skip, "Library has legacy format"
622637
}
623638

624639
if schema.RequiredPropertyMissing("sentence", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -651,9 +666,12 @@ func LibraryPropertiesSentenceFieldSpellCheck() (result checkresult.Type, output
651666

652667
// LibraryPropertiesParagraphFieldMissing checks for missing library.properties "paragraph" field.
653668
func LibraryPropertiesParagraphFieldMissing() (result checkresult.Type, output string) {
654-
shouldRun, reason := runRequiredLibraryPropertiesFieldCheck()
655-
if !shouldRun {
656-
return checkresult.NotRun, reason
669+
if checkdata.LibraryPropertiesLoadError() != nil {
670+
return checkresult.NotRun, "Couldn't load library.properties"
671+
}
672+
673+
if checkdata.LoadedLibrary().IsLegacy {
674+
return checkresult.Skip, "Library has legacy format"
657675
}
658676

659677
if schema.RequiredPropertyMissing("paragraph", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -688,9 +706,12 @@ func LibraryPropertiesParagraphFieldRepeatsSentence() (result checkresult.Type,
688706

689707
// LibraryPropertiesCategoryFieldMissing checks for missing library.properties "category" field.
690708
func LibraryPropertiesCategoryFieldMissing() (result checkresult.Type, output string) {
691-
shouldRun, reason := runRequiredLibraryPropertiesFieldCheck()
692-
if !shouldRun {
693-
return checkresult.NotRun, reason
709+
if checkdata.LibraryPropertiesLoadError() != nil {
710+
return checkresult.NotRun, "Couldn't load library.properties"
711+
}
712+
713+
if checkdata.LoadedLibrary().IsLegacy {
714+
return checkresult.Skip, "Library has legacy format"
694715
}
695716

696717
if schema.RequiredPropertyMissing("category", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -707,7 +728,7 @@ func LibraryPropertiesCategoryFieldInvalid() (result checkresult.Type, output st
707728

708729
category, ok := checkdata.LibraryProperties().GetOk("category")
709730
if !ok {
710-
return checkresult.NotRun, "Field not present"
731+
return checkresult.Skip, "Field not present"
711732
}
712733

713734
if schema.PropertyEnumMismatch("category", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -725,7 +746,7 @@ func LibraryPropertiesCategoryFieldUncategorized() (result checkresult.Type, out
725746

726747
category, ok := checkdata.LibraryProperties().GetOk("category")
727748
if !ok {
728-
return checkresult.NotRun, "Field not present"
749+
return checkresult.Skip, "Field not present"
729750
}
730751

731752
if category == "Uncategorized" {
@@ -737,9 +758,12 @@ func LibraryPropertiesCategoryFieldUncategorized() (result checkresult.Type, out
737758

738759
// LibraryPropertiesUrlFieldMissing checks for missing library.properties "url" field.
739760
func LibraryPropertiesUrlFieldMissing() (result checkresult.Type, output string) {
740-
shouldRun, reason := runRequiredLibraryPropertiesFieldCheck()
741-
if !shouldRun {
742-
return checkresult.NotRun, reason
761+
if checkdata.LibraryPropertiesLoadError() != nil {
762+
return checkresult.NotRun, "Couldn't load library.properties"
763+
}
764+
765+
if checkdata.LoadedLibrary().IsLegacy {
766+
return checkresult.Skip, "Library has legacy format"
743767
}
744768

745769
if schema.RequiredPropertyMissing("url", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -792,9 +816,12 @@ func LibraryPropertiesUrlFieldDeadLink() (result checkresult.Type, output string
792816

793817
// LibraryPropertiesArchitecturesFieldMissing checks for missing library.properties "architectures" field.
794818
func LibraryPropertiesArchitecturesFieldMissing() (result checkresult.Type, output string) {
795-
shouldRun, reason := runRequiredLibraryPropertiesFieldCheck()
796-
if !shouldRun {
797-
return checkresult.NotRun, reason
819+
if checkdata.LibraryPropertiesLoadError() != nil {
820+
return checkresult.NotRun, "Couldn't load library.properties"
821+
}
822+
823+
if checkdata.LoadedLibrary().IsLegacy {
824+
return checkresult.Skip, "Library has legacy format"
798825
}
799826

800827
if schema.RequiredPropertyMissing("architectures", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -810,7 +837,7 @@ func LibraryPropertiesArchitecturesFieldLTMinLength() (result checkresult.Type,
810837
}
811838

812839
if !checkdata.LibraryProperties().ContainsKey("architectures") {
813-
return checkresult.NotRun, "Field not present"
840+
return checkresult.Skip, "Field not present"
814841
}
815842

816843
if schema.PropertyLessThanMinLength("architectures", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -828,7 +855,7 @@ func LibraryPropertiesDependsFieldDisallowedCharacters() (result checkresult.Typ
828855

829856
depends, ok := checkdata.LibraryProperties().GetOk("depends")
830857
if !ok {
831-
return checkresult.NotRun, "Field not present"
858+
return checkresult.Skip, "Field not present"
832859
}
833860

834861
if schema.PropertyPatternMismatch("depends", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -846,7 +873,7 @@ func LibraryPropertiesDependsFieldNotInIndex() (result checkresult.Type, output
846873

847874
depends, hasDepends := checkdata.LibraryProperties().GetOk("depends")
848875
if !hasDepends {
849-
return checkresult.NotRun, "Field not present"
876+
return checkresult.Skip, "Field not present"
850877
}
851878

852879
dependencies := strings.Split(depends, ",")
@@ -878,7 +905,7 @@ func LibraryPropertiesDotALinkageFieldInvalid() (result checkresult.Type, output
878905

879906
dotALinkage, ok := checkdata.LibraryProperties().GetOk("dot_a_linkage")
880907
if !ok {
881-
return checkresult.NotRun, "Field not present"
908+
return checkresult.Skip, "Field not present"
882909
}
883910

884911
if schema.PropertyEnumMismatch("dot_a_linkage", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -895,7 +922,7 @@ func LibraryPropertiesDotALinkageFieldTrueWithFlatLayout() (result checkresult.T
895922
}
896923

897924
if !checkdata.LibraryProperties().ContainsKey("dot_a_linkage") {
898-
return checkresult.NotRun, "Field not present"
925+
return checkresult.Skip, "Field not present"
899926
}
900927

901928
if checkdata.LoadedLibrary().DotALinkage && checkdata.LoadedLibrary().Layout == libraries.FlatLayout {
@@ -912,7 +939,7 @@ func LibraryPropertiesIncludesFieldLTMinLength() (result checkresult.Type, outpu
912939
}
913940

914941
if !checkdata.LibraryProperties().ContainsKey("includes") {
915-
return checkresult.NotRun, "Field not present"
942+
return checkresult.Skip, "Field not present"
916943
}
917944

918945
if schema.PropertyLessThanMinLength("includes", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -930,7 +957,7 @@ func LibraryPropertiesIncludesFieldItemNotFound() (result checkresult.Type, outp
930957

931958
includes, ok := checkdata.LibraryProperties().GetOk("includes")
932959
if !ok {
933-
return checkresult.NotRun, "Field not present"
960+
return checkresult.Skip, "Field not present"
934961
}
935962

936963
includesList := strings.Split(includes, ",")
@@ -972,7 +999,7 @@ func LibraryPropertiesPrecompiledFieldInvalid() (result checkresult.Type, output
972999

9731000
precompiled, ok := checkdata.LibraryProperties().GetOk("precompiled")
9741001
if !ok {
975-
return checkresult.NotRun, "Field not present"
1002+
return checkresult.Skip, "Field not present"
9761003
}
9771004

9781005
if schema.PropertyEnumMismatch("precompiled", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
@@ -990,7 +1017,7 @@ func LibraryPropertiesPrecompiledFieldEnabledWithFlatLayout() (result checkresul
9901017

9911018
precompiled, ok := checkdata.LibraryProperties().GetOk("precompiled")
9921019
if !ok {
993-
return checkresult.NotRun, "Field not present"
1020+
return checkresult.Skip, "Field not present"
9941021
}
9951022

9961023
if checkdata.LoadedLibrary().Precompiled && checkdata.LoadedLibrary().Layout == libraries.FlatLayout {
@@ -1006,6 +1033,10 @@ func LibraryPropertiesLdflagsFieldLTMinLength() (result checkresult.Type, output
10061033
return checkresult.NotRun, "Library not loaded"
10071034
}
10081035

1036+
if !checkdata.LibraryProperties().ContainsKey("ldflags") {
1037+
return checkresult.Skip, "Field not present"
1038+
}
1039+
10091040
if schema.PropertyLessThanMinLength("ldflags", checkdata.LibraryPropertiesSchemaValidationResult()[compliancelevel.Specification], configuration.SchemasPath()) {
10101041
return checkresult.Fail, ""
10111042
}
@@ -1175,7 +1206,7 @@ func LibraryFolderNameGTMaxLength() (result checkresult.Type, output string) {
11751206
func IncorrectLibrarySrcFolderNameCase() (result checkresult.Type, output string) {
11761207
if library.ContainsMetadataFile(checkdata.ProjectPath()) && library.ContainsHeaderFile(checkdata.ProjectPath()) {
11771208
// Flat layout, so no special treatment of src subfolder.
1178-
return checkresult.NotRun, "Not applicable due to layout type"
1209+
return checkresult.Skip, "Not applicable due to layout type"
11791210
}
11801211

11811212
// The library is intended to have the recursive layout.
@@ -1270,7 +1301,7 @@ func RecursiveLibraryWithUtilityFolder() (result checkresult.Type, output string
12701301
}
12711302

12721303
if checkdata.LoadedLibrary().Layout == libraries.FlatLayout {
1273-
return checkresult.NotRun, "Not applicable due to layout type"
1304+
return checkresult.Skip, "Not applicable due to layout type"
12741305
}
12751306

12761307
if checkdata.ProjectPath().Join("utility").Exist() {
@@ -1288,7 +1319,7 @@ func spellCheckLibraryPropertiesFieldValue(fieldName string) (result checkresult
12881319

12891320
fieldValue, ok := checkdata.LibraryProperties().GetOk(fieldName)
12901321
if !ok {
1291-
return checkresult.NotRun, "Field not present"
1322+
return checkresult.Skip, "Field not present"
12921323
}
12931324

12941325
replaced, diff := checkdata.MisspelledWordsReplacer().Replace(fieldValue)
@@ -1327,15 +1358,3 @@ func nameInLibraryManagerIndex(name string) bool {
13271358

13281359
return false
13291360
}
1330-
1331-
func runRequiredLibraryPropertiesFieldCheck() (bool, string) {
1332-
if checkdata.LibraryPropertiesLoadError() != nil {
1333-
return false, "Couldn't load library.properties"
1334-
}
1335-
1336-
if checkdata.LoadedLibrary().IsLegacy {
1337-
return false, "Library has legacy format"
1338-
}
1339-
1340-
return true, ""
1341-
}

0 commit comments

Comments
 (0)