Merge pull request #110 from arduino/per1234/add-checks

Add checks for library.properties architectures field

Author: per1234

Diff for: check/checkconfigurations/checkconfigurations.go

@@ -716,6 +716,36 @@ var configurations = []Type{
 		ErrorModes:      []checkmode.Type{checkmode.Default},
 		CheckFunction:   checkfunctions.LibraryPropertiesArchitecturesFieldLTMinLength,
 	},
+	{
+		ProjectType:     projecttype.Library,
+		Category:        "library.properties",
+		Subcategory:     "architectures field",
+		ID:              "",
+		Brief:           "architecture alias",
+		Description:     "Alternative development frameworks diverged on architecture naming.",
+		MessageTemplate: "Architecture alias(es) in library.properties architectures field: {{.}}. Please also specify the true Arduino architectures compatibilities of the library. See https://arduino.github.io/arduino-cli/latest/library-specification/#libraryproperties-file-format",
+		DisableModes:    nil,
+		EnableModes:     []checkmode.Type{checkmode.Default},
+		InfoModes:       nil,
+		WarningModes:    []checkmode.Type{checkmode.Default},
+		ErrorModes:      []checkmode.Type{checkmode.Strict},
+		CheckFunction:   checkfunctions.LibraryPropertiesArchitecturesFieldSoloAlias,
+	},
+	{
+		ProjectType:     projecttype.Library,
+		Category:        "library.properties",
+		Subcategory:     "architectures field",
+		ID:              "",
+		Brief:           "miscased architecture",
+		Description:     "",
+		MessageTemplate: "Incorrect case of library.properties architectures field item(s): {{.}}. Architectures are case sensitive. See https://arduino.github.io/arduino-cli/latest/library-specification/#libraryproperties-file-format",
+		DisableModes:    nil,
+		EnableModes:     []checkmode.Type{checkmode.Default},
+		InfoModes:       nil,
+		WarningModes:    []checkmode.Type{checkmode.Default},
+		ErrorModes:      []checkmode.Type{checkmode.Strict},
+		CheckFunction:   checkfunctions.LibraryPropertiesArchitecturesFieldValueCase,
+	},
 	{
 		ProjectType:     projecttype.Library,
 		Category:        "library.properties",

Diff for: check/checkfunctions/library.go

@@ -846,6 +846,123 @@ func LibraryPropertiesArchitecturesFieldLTMinLength() (result checkresult.Type,
 	return checkresult.Pass, ""
 }
 
+// LibraryPropertiesArchitecturesFieldAlias checks whether an alias architecture name is present, but not its true Arduino architecture name.
+func LibraryPropertiesArchitecturesFieldSoloAlias() (result checkresult.Type, output string) {
+	if checkdata.LibraryPropertiesLoadError() != nil {
+		return checkresult.NotRun, "Couldn't load library.properties"
+	}
+
+	architectures, ok := checkdata.LibraryProperties().GetOk("architectures")
+	if !ok {
+		return checkresult.Skip, "Field not present"
+	}
+
+	architecturesList := commaSeparatedToList(strings.ToLower(architectures))
+
+	// Must be all lowercase (there is a separate check for incorrect architecture case).
+	var aliases = map[string][]string{
+		"atmelavr":      {"avr"},
+		"atmelmegaavr":  {"megaavr"},
+		"atmelsam":      {"sam", "samd"},
+		"espressif32":   {"esp32"},
+		"espressif8266": {"esp8266"},
+		"intel_arc32":   {"arc32"},
+		"nordicnrf52":   {"nRF5", "nrf52", "mbed"},
+	}
+
+	trueArchitecturePresent := func(trueArchitecturesQuery []string) bool {
+		for _, trueArchitectureQuery := range trueArchitecturesQuery {
+			for _, architecture := range architecturesList {
+				if architecture == trueArchitectureQuery {
+					return true
+				}
+			}
+		}
+
+		return false
+	}
+
+	soloAliases := []string{}
+	for _, architecture := range architecturesList {
+		trueEquivalents, isAlias := aliases[architecture]
+		if isAlias && !trueArchitecturePresent(trueEquivalents) {
+			soloAliases = append(soloAliases, architecture)
+		}
+	}
+
+	if len(soloAliases) > 0 {
+		return checkresult.Fail, strings.Join(soloAliases, ", ")
+	}
+
+	return checkresult.Pass, ""
+}
+
+// LibraryPropertiesArchitecturesFieldValueCase checks for incorrect case of common architectures.
+func LibraryPropertiesArchitecturesFieldValueCase() (result checkresult.Type, output string) {
+	if checkdata.LibraryPropertiesLoadError() != nil {
+		return checkresult.NotRun, "Couldn't load library.properties"
+	}
+
+	architectures, ok := checkdata.LibraryProperties().GetOk("architectures")
+	if !ok {
+		return checkresult.Skip, "Field not present"
+	}
+
+	architecturesList := commaSeparatedToList(architectures)
+
+	var commonArchitecturesList = []string{
+		"apollo3",
+		"arc32",
+		"avr",
+		"esp32",
+		"esp8266",
+		"i586",
+		"i686",
+		"k210",
+		"mbed",
+		"megaavr",
+		"mraa",
+		"nRF5",
+		"nrf52",
+		"pic32",
+		"sam",
+		"samd",
+		"wiced",
+		"win10",
+	}
+
+	correctArchitecturePresent := func(correctArchitectureQuery string) bool {
+		for _, architecture := range architecturesList {
+			if architecture == correctArchitectureQuery {
+				return true
+			}
+		}
+
+		return false
+	}
+
+	miscasedArchitectures := []string{}
+	for _, architecture := range architecturesList {
+		for _, commonArchitecture := range commonArchitecturesList {
+			if architecture == commonArchitecture {
+				break
+			}
+
+			if strings.EqualFold(architecture, commonArchitecture) && !correctArchitecturePresent(commonArchitecture) {
+				// The architecture has incorrect case and the correctly cased name is not present in the architectures field.
+				miscasedArchitectures = append(miscasedArchitectures, architecture)
+				break
+			}
+		}
+	}
+
+	if len(miscasedArchitectures) > 0 {
+		return checkresult.Fail, strings.Join(miscasedArchitectures, ", ")
+	}
+
+	return checkresult.Pass, ""
+}
+
 // LibraryPropertiesDependsFieldDisallowedCharacters checks for disallowed characters in the library.properties "depends" field.
 func LibraryPropertiesDependsFieldDisallowedCharacters() (result checkresult.Type, output string) {
 	if checkdata.LibraryPropertiesLoadError() != nil {
@@ -875,11 +992,10 @@ func LibraryPropertiesDependsFieldNotInIndex() (result checkresult.Type, output
 		return checkresult.Skip, "Field not present"
 	}
 
-	dependencies := strings.Split(depends, ",")
+	dependencies := commaSeparatedToList(depends)
 
 	dependenciesNotInIndex := []string{}
 	for _, dependency := range dependencies {
-		dependency = strings.TrimSpace(dependency)
 		if dependency == "" {
 			continue
 		}
@@ -959,10 +1075,9 @@ func LibraryPropertiesIncludesFieldItemNotFound() (result checkresult.Type, outp
 		return checkresult.Skip, "Field not present"
 	}
 
-	includesList := strings.Split(includes, ",")
+	includesList := commaSeparatedToList(includes)
 
 	findInclude := func(include string) bool {
-		include = strings.TrimSpace(include)
 		if include == "" {
 			return true
 		}
@@ -1357,3 +1472,13 @@ func nameInLibraryManagerIndex(name string) bool {
 
 	return false
 }
+
+// commaSeparatedToList returns the list equivalent of a comma-separated string.
+func commaSeparatedToList(commaSeparated string) []string {
+	list := []string{}
+	for _, item := range strings.Split(commaSeparated, ",") {
+		list = append(list, strings.TrimSpace(item))
+	}
+
+	return list
+}

Diff for: check/checkfunctions/library_test.go

@@ -325,6 +325,30 @@ func TestLibraryPropertiesUrlFieldDeadLink(t *testing.T) {
 	checkLibraryCheckFunction(LibraryPropertiesUrlFieldDeadLink, testTables, t)
 }
 
+func TestLibraryPropertiesArchitecturesFieldSoloAlias(t *testing.T) {
+	testTables := []libraryCheckFunctionTestTable{
+		{"Unable to load", "InvalidLibraryProperties", checkresult.NotRun, ""},
+		{"Not defined", "MissingFields", checkresult.Skip, ""},
+		{"Solo alias", "ArchitectureAliasSolo", checkresult.Fail, ""},
+		{"Alias w/ true", "ArchitectureAliasWithTrue", checkresult.Pass, ""},
+		{"No alias", "Recursive", checkresult.Pass, ""},
+	}
+
+	checkLibraryCheckFunction(LibraryPropertiesArchitecturesFieldSoloAlias, testTables, t)
+}
+
+func TestLibraryPropertiesArchitecturesFieldValueCase(t *testing.T) {
+	testTables := []libraryCheckFunctionTestTable{
+		{"Unable to load", "InvalidLibraryProperties", checkresult.NotRun, ""},
+		{"Not defined", "MissingFields", checkresult.Skip, ""},
+		{"Miscased", "ArchitectureMiscased", checkresult.Fail, ""},
+		{"Miscased w/ correct case", "ArchitectureMiscasedWithCorrect", checkresult.Pass, ""},
+		{"Correct case", "Recursive", checkresult.Pass, ""},
+	}
+
+	checkLibraryCheckFunction(LibraryPropertiesArchitecturesFieldValueCase, testTables, t)
+}
+
 func TestLibraryPropertiesDependsFieldNotInIndex(t *testing.T) {
 	testTables := []libraryCheckFunctionTestTable{
 		{"Unable to load", "InvalidLibraryProperties", checkresult.NotRun, ""},

Diff for: check/checkfunctions/testdata/libraries/ArchitectureAliasSolo/library.properties

@@ -0,0 +1,9 @@
+name=ArchitectureAliasSolo
+version=1.0.0
+author=Cristian Maglie <[email protected]>, Pippo Pluto <[email protected]>
+maintainer=Cristian Maglie <[email protected]>
+sentence=A library that makes coding a web server a breeze.
+paragraph=Supports HTTP1.1 and you can do GET and POST.
+category=Communication
+url=http://example.com/
+architectures=atmelavr, samd

Diff for: check/checkfunctions/testdata/libraries/ArchitectureAliasSolo/src/ArchitectureAliasSolo.h

@@ -0,0 +1,9 @@
+name=ArchitectureAliasWithTrue
+version=1.0.0
+author=Cristian Maglie <[email protected]>, Pippo Pluto <[email protected]>
+maintainer=Cristian Maglie <[email protected]>
+sentence=A library that makes coding a web server a breeze.
+paragraph=Supports HTTP1.1 and you can do GET and POST.
+category=Communication
+url=http://example.com/
+architectures=avr, atmelavr, samd

Diff for: check/checkfunctions/testdata/libraries/ArchitectureAliasWithTrue/library.properties

@@ -0,0 +1,9 @@
+name=ArchitectureMiscased
+version=1.0.0
+author=Cristian Maglie <[email protected]>, Pippo Pluto <[email protected]>
+maintainer=Cristian Maglie <[email protected]>
+sentence=A library that makes coding a web server a breeze.
+paragraph=Supports HTTP1.1 and you can do GET and POST.
+category=Communication
+url=http://example.com/
+architectures=AVR, samd, foo

Diff for: check/checkfunctions/testdata/libraries/ArchitectureAliasWithTrue/src/ArchitectureAliasWithTrue.h

@@ -0,0 +1,9 @@
+name=ArchitectureMiscasedWithCorrect
+version=1.0.0
+author=Cristian Maglie <[email protected]>, Pippo Pluto <[email protected]>
+maintainer=Cristian Maglie <[email protected]>
+sentence=A library that makes coding a web server a breeze.
+paragraph=Supports HTTP1.1 and you can do GET and POST.
+category=Communication
+url=http://example.com/
+architectures=AVR, samd, avr, foo