Add support for version constraints in depends field

per1234 · per1234 · commit cfefa1a625a3 · 2021-06-02T01:32:46.000-07:00
Arduino CLI and the Library Manager indexer has some provisional support for specifying version constraints in the
library.properties depends field. Although undocumented, this syntax is fully supported by the other tools, so Arduino
Lint's previous treatment of libraries using this feature as invalid is incorrect.
diff --git a/etc/schemas/arduino-library-properties-definitions-schema.json b/etc/schemas/arduino-library-properties-definitions-schema.json
@@ -21,13 +21,7 @@
               "allowedCharacters": {
                 "allOf": [
                   {
-                    "$ref": "#/definitions/propertiesObjects/depends/base/definitions/patternObject"
-                  },
-                  {
-                    "not": {
-                      "$comment": "The depends property is a comma separated list of names, so a valid name pattern is a valid depends pattern with the comma excluded",
-                      "pattern": "^.*,.*$"
-                    }
+                    "pattern": "^(([a-zA-Z][a-zA-Z0-9 _.\\-]*)|([0-9][a-zA-Z0-9 _.\\-]*[a-zA-Z][a-zA-Z0-9 _.\\-]*))*$"
                   }
                 ]
               }
@@ -494,18 +488,13 @@
       },
       "depends": {
         "base": {
-          "definitions": {
-            "patternObject": {
-              "pattern": "^(([a-zA-Z][a-zA-Z0-9 _\\.\\-,]*)|([0-9][a-zA-Z0-9 _\\.\\-]*[a-zA-Z][a-zA-Z0-9 _\\.\\-,]*))*$"
-            }
-          },
           "object": {
             "allOf": [
               {
                 "type": "string"
               },
               {
-                "$ref": "#/definitions/propertiesObjects/depends/base/definitions/patternObject"
+                "pattern": "^((((([a-zA-Z][a-zA-Z0-9 _.\\-]*)|([0-9][a-zA-Z0-9 _.\\-]*[a-zA-Z][a-zA-Z0-9 _.\\-]*))+( \\( *(<|<=|=|>=|>)(0|[1-9]\\d*)(\\.(0|[1-9]\\d*))?(\\.(0|[1-9]\\d*))?(-((0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(\\.(0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(\\+([0-9a-zA-Z-]+(\\.[0-9a-zA-Z-]+)*))? *\\) *)?), *)*((([a-zA-Z][a-zA-Z0-9 _.\\-]*)|([0-9][a-zA-Z0-9 _.\\-]*[a-zA-Z][a-zA-Z0-9 _.\\-]*))+( \\( *(<|<=|=|>=|>)(0|[1-9]\\d*)(\\.(0|[1-9]\\d*))?(\\.(0|[1-9]\\d*))?(-((0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(\\.(0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(\\+([0-9a-zA-Z-]+(\\.[0-9a-zA-Z-]+)*))? *\\) *)?))?$"
               }
             ]
           }
diff --git a/internal/project/library/libraryproperties/librarypropertiesschemas_test.go b/internal/project/library/libraryproperties/librarypropertiesschemas_test.go
@@ -330,13 +330,60 @@ func TestPropertiesUrlFormat(t *testing.T) {
 
 func TestPropertiesDependsPattern(t *testing.T) {
 	testTables := []propertyValueTestTable{
+		{"Valid name", "foo", compliancelevel.Permissive, assert.False},
+		{"Valid name", "foo", compliancelevel.Specification, assert.False},
+		{"Valid name", "foo", compliancelevel.Strict, assert.False},
+
+		{"Valid names", "foo,bar", compliancelevel.Permissive, assert.False},
+		{"Valid names", "foo,bar", compliancelevel.Specification, assert.False},
+		{"Valid names", "foo,bar", compliancelevel.Strict, assert.False},
+
+		{"Trailing comma", "foo,", compliancelevel.Permissive, assert.True},
+		{"Trailing comma", "foo,", compliancelevel.Specification, assert.True},
+		{"Trailing comma", "foo,", compliancelevel.Strict, assert.True},
+
 		{"Invalid characters", "-foo", compliancelevel.Permissive, assert.True},
 		{"Invalid characters", "-foo", compliancelevel.Specification, assert.True},
 		{"Invalid characters", "-foo", compliancelevel.Strict, assert.True},
 
 		{"Empty", "", compliancelevel.Permissive, assert.False},
 		{"Empty", "", compliancelevel.Specification, assert.False},
 		{"Empty", "", compliancelevel.Strict, assert.False},
+
+		{"<version", "foo (<1.2.3)", compliancelevel.Permissive, assert.False},
+		{"<version", "foo (<1.2.3)", compliancelevel.Specification, assert.False},
+		{"<version", "foo (<1.2.3)", compliancelevel.Strict, assert.False},
+		{"<=version", "foo (<=1.2.3)", compliancelevel.Permissive, assert.False},
+		{"<=version", "foo (<=1.2.3)", compliancelevel.Specification, assert.False},
+		{"<=version", "foo (<=1.2.3)", compliancelevel.Strict, assert.False},
+		{"=version", "foo (=1.2.3)", compliancelevel.Permissive, assert.False},
+		{"=version", "foo (=1.2.3)", compliancelevel.Specification, assert.False},
+		{"=version", "foo (=1.2.3)", compliancelevel.Strict, assert.False},
+		{">=version", "foo (>=1.2.3)", compliancelevel.Permissive, assert.False},
+		{">=version", "foo (>=1.2.3)", compliancelevel.Specification, assert.False},
+		{">=version", "foo (>=1.2.3)", compliancelevel.Strict, assert.False},
+		{">version", "foo (>1.2.3)", compliancelevel.Permissive, assert.False},
+		{">version", "foo (>1.2.3)", compliancelevel.Specification, assert.False},
+		{">version", "foo (>1.2.3)", compliancelevel.Strict, assert.False},
+
+		{"Relaxed version", "foo (=1.2)", compliancelevel.Permissive, assert.False},
+		{"Relaxed version", "foo (=1.2)", compliancelevel.Specification, assert.False},
+		{"Relaxed version", "foo (=1.2)", compliancelevel.Strict, assert.False},
+		{"Pre-release version", "foo (=1.2.3-rc1)", compliancelevel.Permissive, assert.False},
+		{"Pre-release version", "foo (=1.2.3-rc1)", compliancelevel.Specification, assert.False},
+		{"Pre-release version", "foo (=1.2.3-rc1)", compliancelevel.Strict, assert.False},
+
+		{"Invalid version", "foo (bar)", compliancelevel.Permissive, assert.True},
+		{"Invalid version", "foo (bar)", compliancelevel.Specification, assert.True},
+		{"Invalid version", "foo (bar)", compliancelevel.Strict, assert.True},
+
+		{"Version w/o space", "foo(>1.2.3)", compliancelevel.Permissive, assert.True},
+		{"Version w/o space", "foo(>1.2.3)", compliancelevel.Specification, assert.True},
+		{"Version w/o space", "foo(>1.2.3)", compliancelevel.Strict, assert.True},
+
+		{"Names w/ version", "foo (<=1.2.3),bar", compliancelevel.Permissive, assert.False},
+		{"Names w/ version", "foo (<=1.2.3),bar", compliancelevel.Specification, assert.False},
+		{"Names w/ version", "foo (<=1.2.3),bar", compliancelevel.Strict, assert.False},
 	}
 
 	checkPropertyPatternMismatch("depends", testTables, t)
diff --git a/internal/rule/ruleconfiguration/ruleconfiguration.go b/internal/rule/ruleconfiguration/ruleconfiguration.go
@@ -1007,15 +1007,15 @@ var configurations = []Type{
 		Category:         "library.properties",
 		Subcategory:      "depends field",
 		ID:               "LP047",
-		Brief:            "prohibited character in depends",
+		Brief:            "invalid depends format",
 		Description:      "",
-		MessageTemplate:  "Prohibited character(s) in library.properties depends field {{.}}. See: https://arduino.github.io/arduino-cli/latest/library-specification/#libraryproperties-file-format",
+		MessageTemplate:  "Invalid format of library.properties depends field {{.}}. See: https://arduino.github.io/arduino-cli/latest/library-specification/#libraryproperties-file-format",
 		DisableModes:     nil,
 		EnableModes:      []rulemode.Type{rulemode.Default},
 		InfoModes:        nil,
 		WarningModes:     nil,
 		ErrorModes:       []rulemode.Type{rulemode.Default},
-		RuleFunction:     rulefunction.LibraryPropertiesDependsFieldDisallowedCharacters,
+		RuleFunction:     rulefunction.LibraryPropertiesDependsFieldInvalidFormat,
 	},
 	{
 		ProjectType:      projecttype.Library,
diff --git a/internal/rule/rulefunction/library.go b/internal/rule/rulefunction/library.go
@@ -1158,8 +1158,8 @@ func LibraryPropertiesArchitecturesFieldValueCase() (result ruleresult.Type, out
 	return ruleresult.Pass, ""
 }
 
-// LibraryPropertiesDependsFieldDisallowedCharacters checks for disallowed characters in the library.properties "depends" field.
-func LibraryPropertiesDependsFieldDisallowedCharacters() (result ruleresult.Type, output string) {
+// LibraryPropertiesDependsFieldInvalidFormat checks for disallowed characters in the library.properties "depends" field.
+func LibraryPropertiesDependsFieldInvalidFormat() (result ruleresult.Type, output string) {
 	if projectdata.LibraryPropertiesLoadError() != nil {
 		return ruleresult.NotRun, "Couldn't load library.properties"
 	}
diff --git a/internal/rule/rulefunction/library_test.go b/internal/rule/rulefunction/library_test.go
@@ -794,15 +794,15 @@ func TestLibraryPropertiesArchitecturesFieldValueCase(t *testing.T) {
 	checkLibraryRuleFunction(LibraryPropertiesArchitecturesFieldValueCase, testTables, t)
 }
 
-func TestLibraryPropertiesDependsFieldDisallowedCharacters(t *testing.T) {
+func TestLibraryPropertiesDependsFieldInvalidFormat(t *testing.T) {
 	testTables := []libraryRuleFunctionTestTable{
 		{"Invalid", "InvalidLibraryProperties", ruleresult.NotRun, ""},
 		{"Legacy", "Legacy", ruleresult.Skip, ""},
 		{"Depends field has disallowed characters", "DependsHasBadChars", ruleresult.Fail, ""},
-		{"Valid", "DependsIndexed", ruleresult.Pass, ""},
+		{"Valid", "DependsValid", ruleresult.Pass, ""},
 	}
 
-	checkLibraryRuleFunction(LibraryPropertiesDependsFieldDisallowedCharacters, testTables, t)
+	checkLibraryRuleFunction(LibraryPropertiesDependsFieldInvalidFormat, testTables, t)
 }
 
 func TestLibraryPropertiesDependsFieldNotInIndex(t *testing.T) {
diff --git a/internal/rule/rulefunction/testdata/libraries/DependsValid/library.properties b/internal/rule/rulefunction/testdata/libraries/DependsValid/library.properties
@@ -0,0 +1,11 @@
+name=DependsValid
+version=1.0.0
+author=Cristian Maglie <c.maglie@example.com>, Pippo Pluto <pippo@example.com>
+maintainer=Cristian Maglie <c.maglie@example.com>
+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
+depends=Servo , Adafruit NeoPixel,Stepper (<1.1.3) , Mouse (<=1.0.0),Keyboard  (=1.2.3-beta),Ethernet (>=2.0),WiFiNINA (>0.0.8)
+includes=DependsValid.h
diff --git a/internal/rule/rulefunction/testdata/libraries/DependsValid/src/DependsValid.h b/internal/rule/rulefunction/testdata/libraries/DependsValid/src/DependsValid.h
diff --git a/internal/rule/schema/schemadata/bindata.go b/internal/rule/schema/schemadata/bindata.go
@@ -1440,13 +1440,7 @@ var _arduinoLibraryPropertiesDefinitionsSchemaJson = []byte(`{
               "allowedCharacters": {
                 "allOf": [
                   {
-                    "$ref": "#/definitions/propertiesObjects/depends/base/definitions/patternObject"
-                  },
-                  {
-                    "not": {
-                      "$comment": "The depends property is a comma separated list of names, so a valid name pattern is a valid depends pattern with the comma excluded",
-                      "pattern": "^.*,.*$"
-                    }
+                    "pattern": "^(([a-zA-Z][a-zA-Z0-9 _.\\-]*)|([0-9][a-zA-Z0-9 _.\\-]*[a-zA-Z][a-zA-Z0-9 _.\\-]*))*$"
                   }
                 ]
               }
@@ -1913,18 +1907,13 @@ var _arduinoLibraryPropertiesDefinitionsSchemaJson = []byte(`{
       },
       "depends": {
         "base": {
-          "definitions": {
-            "patternObject": {
-              "pattern": "^(([a-zA-Z][a-zA-Z0-9 _\\.\\-,]*)|([0-9][a-zA-Z0-9 _\\.\\-]*[a-zA-Z][a-zA-Z0-9 _\\.\\-,]*))*$"
-            }
-          },
           "object": {
             "allOf": [
               {
                 "type": "string"
               },
               {
-                "$ref": "#/definitions/propertiesObjects/depends/base/definitions/patternObject"
+                "pattern": "^((((([a-zA-Z][a-zA-Z0-9 _.\\-]*)|([0-9][a-zA-Z0-9 _.\\-]*[a-zA-Z][a-zA-Z0-9 _.\\-]*))+( \\( *(<|<=|=|>=|>)(0|[1-9]\\d*)(\\.(0|[1-9]\\d*))?(\\.(0|[1-9]\\d*))?(-((0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(\\.(0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(\\+([0-9a-zA-Z-]+(\\.[0-9a-zA-Z-]+)*))? *\\) *)?), *)*((([a-zA-Z][a-zA-Z0-9 _.\\-]*)|([0-9][a-zA-Z0-9 _.\\-]*[a-zA-Z][a-zA-Z0-9 _.\\-]*))+( \\( *(<|<=|=|>=|>)(0|[1-9]\\d*)(\\.(0|[1-9]\\d*))?(\\.(0|[1-9]\\d*))?(-((0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(\\.(0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(\\+([0-9a-zA-Z-]+(\\.[0-9a-zA-Z-]+)*))? *\\) *)?))?$"
               }
             ]
           }

Original file line number	Diff line number	Diff line change
`@@ -21,13 +21,7 @@`
`21`	`21`	`"allowedCharacters": {`
`22`	`22`	`"allOf": [`
`23`	`23`	`{`
`24`		`- "$ref": "#/definitions/propertiesObjects/depends/base/definitions/patternObject"`
`25`		`- },`
`26`		`- {`
`27`		`- "not": {`
`28`		`- "$comment": "The depends property is a comma separated list of names, so a valid name pattern is a valid depends pattern with the comma excluded",`
`29`		`- "pattern": "^.,.$"`
`30`		`- }`
	`24`	`+ "pattern": "^(([a-zA-Z][a-zA-Z0-9 _.\\-])\|([0-9][a-zA-Z0-9 _.\\-][a-zA-Z][a-zA-Z0-9 _.\\-]))$"`
`31`	`25`	`}`
`32`	`26`	`]`
`33`	`27`	`}`
`@@ -494,18 +488,13 @@`
`494`	`488`	`},`
`495`	`489`	`"depends": {`
`496`	`490`	`"base": {`
`497`		`- "definitions": {`
`498`		`- "patternObject": {`
`499`		`- "pattern": "^(([a-zA-Z][a-zA-Z0-9 _\\.\\-,])\|([0-9][a-zA-Z0-9 _\\.\\-][a-zA-Z][a-zA-Z0-9 _\\.\\-,]))$"`
`500`		`- }`
`501`		`- },`
`502`	`491`	`"object": {`
`503`	`492`	`"allOf": [`
`504`	`493`	`{`
`505`	`494`	`"type": "string"`
`506`	`495`	`},`
`507`	`496`	`{`
`508`		`- "$ref": "#/definitions/propertiesObjects/depends/base/definitions/patternObject"`
	`497`	+ "pattern": "^((((([a-zA-Z][a-zA-Z0-9 _.\\-])\|([0-9][a-zA-Z0-9 _.\\-][a-zA-Z][a-zA-Z0-9 _.\\-]))+( \\( (<\|<=\|=\|>=\|>)(0\|[1-9]\\d)(\\.(0\|[1-9]\\d))?(\\.(0\|[1-9]\\d))?(-((0\|[1-9]\\d\|\\d[a-zA-Z-][0-9a-zA-Z-])(\\.(0\|[1-9]\\d\|\\d[a-zA-Z-][0-9a-zA-Z-]))))?(\\+([0-9a-zA-Z-]+(\\.[0-9a-zA-Z-]+)))? \\) )?), )((([a-zA-Z][a-zA-Z0-9 _.\\-])\|([0-9][a-zA-Z0-9 _.\\-][a-zA-Z][a-zA-Z0-9 _.\\-]))+( \\( (<\|<=\|=\|>=\|>)(0\|[1-9]\\d)(\\.(0\|[1-9]\\d))?(\\.(0\|[1-9]\\d))?(-((0\|[1-9]\\d\|\\d[a-zA-Z-][0-9a-zA-Z-])(\\.(0\|[1-9]\\d\|\\d[a-zA-Z-][0-9a-zA-Z-]))))?(\\+([0-9a-zA-Z-]+(\\.[0-9a-zA-Z-]+)))? \\) )?))?$"
`509`	`498`	`}`
`510`	`499`	`]`
`511`	`500`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1158,8 +1158,8 @@ func LibraryPropertiesArchitecturesFieldValueCase() (result ruleresult.Type, out`
`1158`	`1158`	`return ruleresult.Pass, ""`
`1159`	`1159`	`}`
`1160`	`1160`
`1161`		`-// LibraryPropertiesDependsFieldDisallowedCharacters checks for disallowed characters in the library.properties "depends" field.`
`1162`		`-func LibraryPropertiesDependsFieldDisallowedCharacters() (result ruleresult.Type, output string) {`
	`1161`	`+// LibraryPropertiesDependsFieldInvalidFormat checks for disallowed characters in the library.properties "depends" field.`
	`1162`	`+func LibraryPropertiesDependsFieldInvalidFormat() (result ruleresult.Type, output string) {`
`1163`	`1163`	`if projectdata.LibraryPropertiesLoadError() != nil {`
`1164`	`1164`	`return ruleresult.NotRun, "Couldn't load library.properties"`
`1165`	`1165`	`}`
Original file line number	Diff line number	Diff line change
`@@ -794,15 +794,15 @@ func TestLibraryPropertiesArchitecturesFieldValueCase(t *testing.T) {`
`794`	`794`	`checkLibraryRuleFunction(LibraryPropertiesArchitecturesFieldValueCase, testTables, t)`
`795`	`795`	`}`
`796`	`796`
`797`		`-func TestLibraryPropertiesDependsFieldDisallowedCharacters(t *testing.T) {`
	`797`	`+func TestLibraryPropertiesDependsFieldInvalidFormat(t *testing.T) {`
`798`	`798`	`testTables := []libraryRuleFunctionTestTable{`
`799`	`799`	`{"Invalid", "InvalidLibraryProperties", ruleresult.NotRun, ""},`
`800`	`800`	`{"Legacy", "Legacy", ruleresult.Skip, ""},`
`801`	`801`	`{"Depends field has disallowed characters", "DependsHasBadChars", ruleresult.Fail, ""},`
`802`		`- {"Valid", "DependsIndexed", ruleresult.Pass, ""},`
	`802`	`+ {"Valid", "DependsValid", ruleresult.Pass, ""},`
`803`	`803`	`}`
`804`	`804`
`805`		`- checkLibraryRuleFunction(LibraryPropertiesDependsFieldDisallowedCharacters, testTables, t)`
	`805`	`+ checkLibraryRuleFunction(LibraryPropertiesDependsFieldInvalidFormat, testTables, t)`
`806`	`806`	`}`
`807`	`807`
`808`	`808`	`func TestLibraryPropertiesDependsFieldNotInIndex(t *testing.T) {`
Original file line number	Diff line number	Diff line change
@@ -1440,13 +1440,7 @@ var _arduinoLibraryPropertiesDefinitionsSchemaJson = []byte(`{
`1440`	`1440`	`"allowedCharacters": {`
`1441`	`1441`	`"allOf": [`
`1442`	`1442`	`{`
`1443`		`- "$ref": "#/definitions/propertiesObjects/depends/base/definitions/patternObject"`
`1444`		`- },`
`1445`		`- {`
`1446`		`- "not": {`
`1447`		`- "$comment": "The depends property is a comma separated list of names, so a valid name pattern is a valid depends pattern with the comma excluded",`
`1448`		`- "pattern": "^.,.$"`
`1449`		`- }`
	`1443`	`+ "pattern": "^(([a-zA-Z][a-zA-Z0-9 _.\\-])\|([0-9][a-zA-Z0-9 _.\\-][a-zA-Z][a-zA-Z0-9 _.\\-]))$"`
`1450`	`1444`	`}`
`1451`	`1445`	`]`
`1452`	`1446`	`}`
@@ -1913,18 +1907,13 @@ var _arduinoLibraryPropertiesDefinitionsSchemaJson = []byte(`{
`1913`	`1907`	`},`
`1914`	`1908`	`"depends": {`
`1915`	`1909`	`"base": {`
`1916`		`- "definitions": {`
`1917`		`- "patternObject": {`
`1918`		`- "pattern": "^(([a-zA-Z][a-zA-Z0-9 _\\.\\-,])\|([0-9][a-zA-Z0-9 _\\.\\-][a-zA-Z][a-zA-Z0-9 _\\.\\-,]))$"`
`1919`		`- }`
`1920`		`- },`
`1921`	`1910`	`"object": {`
`1922`	`1911`	`"allOf": [`
`1923`	`1912`	`{`
`1924`	`1913`	`"type": "string"`
`1925`	`1914`	`},`
`1926`	`1915`	`{`
`1927`		`- "$ref": "#/definitions/propertiesObjects/depends/base/definitions/patternObject"`
	`1916`	+ "pattern": "^((((([a-zA-Z][a-zA-Z0-9 _.\\-])\|([0-9][a-zA-Z0-9 _.\\-][a-zA-Z][a-zA-Z0-9 _.\\-]))+( \\( (<\|<=\|=\|>=\|>)(0\|[1-9]\\d)(\\.(0\|[1-9]\\d))?(\\.(0\|[1-9]\\d))?(-((0\|[1-9]\\d\|\\d[a-zA-Z-][0-9a-zA-Z-])(\\.(0\|[1-9]\\d\|\\d[a-zA-Z-][0-9a-zA-Z-]))))?(\\+([0-9a-zA-Z-]+(\\.[0-9a-zA-Z-]+)))? \\) )?), )((([a-zA-Z][a-zA-Z0-9 _.\\-])\|([0-9][a-zA-Z0-9 _.\\-][a-zA-Z][a-zA-Z0-9 _.\\-]))+( \\( (<\|<=\|=\|>=\|>)(0\|[1-9]\\d)(\\.(0\|[1-9]\\d))?(\\.(0\|[1-9]\\d))?(-((0\|[1-9]\\d\|\\d[a-zA-Z-][0-9a-zA-Z-])(\\.(0\|[1-9]\\d\|\\d[a-zA-Z-][0-9a-zA-Z-]))))?(\\+([0-9a-zA-Z-]+(\\.[0-9a-zA-Z-]+)))? \\) )?))?$"
`1928`	`1917`	`}`
`1929`	`1918`	`]`
`1930`	`1919`	`}`