Skip to content

Commit cb7776a

Browse files
per1234per1234
authored
Merge pull request #182 from per1234/depends-constraint
Add support for version constraints in depends field
2 parents 45b32ff + 4674bc7 commit cb7776a

File tree

12 files changed

+158
-65
lines changed

12 files changed

+158
-65
lines changed

‎etc/schemas/arduino-library-properties-definitions-schema.json‎

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,7 @@
2121
"allowedCharacters": {
2222
"allOf": [
2323
{
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 _.\\-]*))*$"
3125
}
3226
]
3327
}
@@ -494,18 +488,14 @@
494488
},
495489
"depends": {
496490
"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-
},
502491
"object": {
503492
"allOf": [
504493
{
505494
"type": "string"
506495
},
507496
{
508-
"$ref": "#/definitions/propertiesObjects/depends/base/definitions/patternObject"
497+
"$comment": "Based on #/definitions/propertiesObjects/name/base/definitions/patternObjects/allowedCharacters and general-definitions-schema.json#/definitions/patternObjects/relaxedSemver",
498+
"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-]+)*))? *\\) *)?))?$"
509499
}
510500
]
511501
}

‎internal/project/library/libraryproperties/librarypropertiesschemas_test.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,13 +330,60 @@ func TestPropertiesUrlFormat(t *testing.T) {
330330

331331
func TestPropertiesDependsPattern(t *testing.T) {
332332
testTables := []propertyValueTestTable{
333+
{"Valid name", "foo", compliancelevel.Permissive, assert.False},
334+
{"Valid name", "foo", compliancelevel.Specification, assert.False},
335+
{"Valid name", "foo", compliancelevel.Strict, assert.False},
336+
337+
{"Valid names", "foo,bar", compliancelevel.Permissive, assert.False},
338+
{"Valid names", "foo,bar", compliancelevel.Specification, assert.False},
339+
{"Valid names", "foo,bar", compliancelevel.Strict, assert.False},
340+
341+
{"Trailing comma", "foo,", compliancelevel.Permissive, assert.True},
342+
{"Trailing comma", "foo,", compliancelevel.Specification, assert.True},
343+
{"Trailing comma", "foo,", compliancelevel.Strict, assert.True},
344+
333345
{"Invalid characters", "-foo", compliancelevel.Permissive, assert.True},
334346
{"Invalid characters", "-foo", compliancelevel.Specification, assert.True},
335347
{"Invalid characters", "-foo", compliancelevel.Strict, assert.True},
336348

337349
{"Empty", "", compliancelevel.Permissive, assert.False},
338350
{"Empty", "", compliancelevel.Specification, assert.False},
339351
{"Empty", "", compliancelevel.Strict, assert.False},
352+
353+
{"<version", "foo (<1.2.3)", compliancelevel.Permissive, assert.False},
354+
{"<version", "foo (<1.2.3)", compliancelevel.Specification, assert.False},
355+
{"<version", "foo (<1.2.3)", compliancelevel.Strict, assert.False},
356+
{"<=version", "foo (<=1.2.3)", compliancelevel.Permissive, assert.False},
357+
{"<=version", "foo (<=1.2.3)", compliancelevel.Specification, assert.False},
358+
{"<=version", "foo (<=1.2.3)", compliancelevel.Strict, assert.False},
359+
{"=version", "foo (=1.2.3)", compliancelevel.Permissive, assert.False},
360+
{"=version", "foo (=1.2.3)", compliancelevel.Specification, assert.False},
361+
{"=version", "foo (=1.2.3)", compliancelevel.Strict, assert.False},
362+
{">=version", "foo (>=1.2.3)", compliancelevel.Permissive, assert.False},
363+
{">=version", "foo (>=1.2.3)", compliancelevel.Specification, assert.False},
364+
{">=version", "foo (>=1.2.3)", compliancelevel.Strict, assert.False},
365+
{">version", "foo (>1.2.3)", compliancelevel.Permissive, assert.False},
366+
{">version", "foo (>1.2.3)", compliancelevel.Specification, assert.False},
367+
{">version", "foo (>1.2.3)", compliancelevel.Strict, assert.False},
368+
369+
{"Relaxed version", "foo (=1.2)", compliancelevel.Permissive, assert.False},
370+
{"Relaxed version", "foo (=1.2)", compliancelevel.Specification, assert.False},
371+
{"Relaxed version", "foo (=1.2)", compliancelevel.Strict, assert.False},
372+
{"Pre-release version", "foo (=1.2.3-rc1)", compliancelevel.Permissive, assert.False},
373+
{"Pre-release version", "foo (=1.2.3-rc1)", compliancelevel.Specification, assert.False},
374+
{"Pre-release version", "foo (=1.2.3-rc1)", compliancelevel.Strict, assert.False},
375+
376+
{"Invalid version", "foo (bar)", compliancelevel.Permissive, assert.True},
377+
{"Invalid version", "foo (bar)", compliancelevel.Specification, assert.True},
378+
{"Invalid version", "foo (bar)", compliancelevel.Strict, assert.True},
379+
380+
{"Version w/o space", "foo(>1.2.3)", compliancelevel.Permissive, assert.True},
381+
{"Version w/o space", "foo(>1.2.3)", compliancelevel.Specification, assert.True},
382+
{"Version w/o space", "foo(>1.2.3)", compliancelevel.Strict, assert.True},
383+
384+
{"Names w/ version", "foo (<=1.2.3),bar", compliancelevel.Permissive, assert.False},
385+
{"Names w/ version", "foo (<=1.2.3),bar", compliancelevel.Specification, assert.False},
386+
{"Names w/ version", "foo (<=1.2.3),bar", compliancelevel.Strict, assert.False},
340387
}
341388

342389
checkPropertyPatternMismatch("depends", testTables, t)

‎internal/project/projectdata/library.go‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,20 @@
1616
package projectdata
1717

1818
import (
19-
"encoding/json"
20-
"io/ioutil"
19+
"io"
2120
"net/http"
2221
"os"
2322

2423
"github.com/arduino/arduino-cli/arduino/libraries"
24+
"github.com/arduino/arduino-cli/arduino/libraries/librariesmanager"
2525
"github.com/arduino/arduino-lint/internal/configuration"
2626
"github.com/arduino/arduino-lint/internal/configuration/rulemode"
2727
"github.com/arduino/arduino-lint/internal/project"
2828
"github.com/arduino/arduino-lint/internal/project/library/libraryproperties"
2929
"github.com/arduino/arduino-lint/internal/result/feedback"
3030
"github.com/arduino/arduino-lint/internal/rule/schema"
3131
"github.com/arduino/arduino-lint/internal/rule/schema/compliancelevel"
32+
"github.com/arduino/go-paths-helper"
3233
"github.com/arduino/go-properties-orderedmap"
3334
"github.com/client9/misspell"
3435
"github.com/sirupsen/logrus"
@@ -60,23 +61,34 @@ func InitializeForLibrary(project project.Type) {
6061

6162
// Download the Library Manager index if needed.
6263
if !configuration.RuleModes(project.SuperprojectType)[rulemode.LibraryManagerIndexing] && libraryManagerIndex == nil {
63-
url := "http://downloads.arduino.cc/libraries/library_index.json"
64-
httpResponse, err := http.Get(url)
64+
// Set up the temporary folder for the index
65+
libraryIndexFolderPath, err := paths.TempDir().MkTempDir("arduino-lint-library-index-folder")
66+
defer libraryIndexFolderPath.RemoveAll()
6567
if err != nil {
66-
feedback.Errorf("Unable to download Library Manager index from %s: %s", err, url)
68+
panic(err)
69+
}
70+
libraryIndexPath := libraryIndexFolderPath.Join("library_index.json")
71+
72+
// Download the index data
73+
httpResponse, err := http.Get(librariesmanager.LibraryIndexURL.String())
74+
if err != nil {
75+
feedback.Errorf("Unable to download Library Manager index from %s: %s", err, librariesmanager.LibraryIndexURL)
6776
os.Exit(1)
6877
}
6978
defer httpResponse.Body.Close()
7079

71-
bytes, err := ioutil.ReadAll(httpResponse.Body)
80+
// Write the index data to file
81+
libraryIndexFile, err := libraryIndexPath.Create()
82+
defer libraryIndexFile.Close()
7283
if err != nil {
7384
panic(err)
7485
}
75-
76-
err = json.Unmarshal(bytes, &libraryManagerIndex)
77-
if err != nil {
86+
if _, err := io.Copy(libraryIndexFile, httpResponse.Body); err != nil {
7887
panic(err)
7988
}
89+
90+
libraryManagerIndex = librariesmanager.NewLibraryManager(libraryIndexFolderPath, nil)
91+
libraryManagerIndex.LoadIndex()
8092
}
8193

8294
if misspelledWordsReplacer == nil { // The replacer only needs to be compiled once per run.
@@ -120,10 +132,10 @@ func SourceHeaders() []string {
120132
return sourceHeaders
121133
}
122134

123-
var libraryManagerIndex map[string]interface{}
135+
var libraryManagerIndex *librariesmanager.LibrariesManager
124136

125137
// LibraryManagerIndex returns the Library Manager index data.
126-
func LibraryManagerIndex() map[string]interface{} {
138+
func LibraryManagerIndex() *librariesmanager.LibrariesManager {
127139
return libraryManagerIndex
128140
}
129141

‎internal/rule/ruleconfiguration/ruleconfiguration.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,15 +1007,15 @@ var configurations = []Type{
10071007
Category: "library.properties",
10081008
Subcategory: "depends field",
10091009
ID: "LP047",
1010-
Brief: "prohibited character in depends",
1010+
Brief: "invalid depends format",
10111011
Description: "",
1012-
MessageTemplate: "Prohibited character(s) in library.properties depends field {{.}}. See: https://arduino.github.io/arduino-cli/latest/library-specification/#libraryproperties-file-format",
1012+
MessageTemplate: "Invalid format of library.properties depends field {{.}}. See: https://arduino.github.io/arduino-cli/latest/library-specification/#libraryproperties-file-format",
10131013
DisableModes: nil,
10141014
EnableModes: []rulemode.Type{rulemode.Default},
10151015
InfoModes: nil,
10161016
WarningModes: nil,
10171017
ErrorModes: []rulemode.Type{rulemode.Default},
1018-
RuleFunction: rulefunction.LibraryPropertiesDependsFieldDisallowedCharacters,
1018+
RuleFunction: rulefunction.LibraryPropertiesDependsFieldInvalidFormat,
10191019
},
10201020
{
10211021
ProjectType: projecttype.Library,

‎internal/rule/rulefunction/library.go‎

Lines changed: 49 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,11 @@ import (
2222
"net/http"
2323
"os"
2424
"path/filepath"
25+
"regexp"
2526
"strings"
2627

2728
"github.com/arduino/arduino-cli/arduino/libraries"
29+
"github.com/arduino/arduino-cli/arduino/libraries/librariesindex"
2830
"github.com/arduino/arduino-cli/arduino/utils"
2931
"github.com/arduino/arduino-lint/internal/project/library"
3032
"github.com/arduino/arduino-lint/internal/project/projectdata"
@@ -1158,8 +1160,8 @@ func LibraryPropertiesArchitecturesFieldValueCase() (result ruleresult.Type, out
11581160
return ruleresult.Pass, ""
11591161
}
11601162

1161-
// LibraryPropertiesDependsFieldDisallowedCharacters checks for disallowed characters in the library.properties "depends" field.
1162-
func LibraryPropertiesDependsFieldDisallowedCharacters() (result ruleresult.Type, output string) {
1163+
// LibraryPropertiesDependsFieldInvalidFormat checks for the library.properties "depends" field having an invalid format.
1164+
func LibraryPropertiesDependsFieldInvalidFormat() (result ruleresult.Type, output string) {
11631165
if projectdata.LibraryPropertiesLoadError() != nil {
11641166
return ruleresult.NotRun, "Couldn't load library.properties"
11651167
}
@@ -1187,21 +1189,55 @@ func LibraryPropertiesDependsFieldNotInIndex() (result ruleresult.Type, output s
11871189
return ruleresult.Skip, "Field not present"
11881190
}
11891191

1190-
dependencies := commaSeparatedToList(depends)
1192+
dependsList := commaSeparatedToList(depends)
11911193

1192-
dependenciesNotInIndex := []string{}
1193-
for _, dependency := range dependencies {
1194-
if dependency == "" {
1194+
var dependencyRegexp = regexp.MustCompile("^([^()]+?) *(?:\\((.+)\\))?$")
1195+
dependsNotInIndex := []string{}
1196+
for _, depend := range dependsList {
1197+
// Process raw depend string into a dependency object
1198+
if depend == "" {
1199+
// This is the responsibility of LibraryPropertiesDependsFieldInvalidFormat()
11951200
continue
11961201
}
1197-
logrus.Tracef("Checking if dependency %s is in index.", dependency)
1198-
if !nameInLibraryManagerIndex(dependency) {
1199-
dependenciesNotInIndex = append(dependenciesNotInIndex, dependency)
1202+
dependencyData := dependencyRegexp.FindAllStringSubmatch(depend, -1)
1203+
if dependencyData == nil {
1204+
// This is the responsibility of LibraryPropertiesDependsFieldInvalidFormat()
1205+
continue
1206+
}
1207+
dependencyConstraint, err := semver.ParseConstraint(dependencyData[0][2])
1208+
if err != nil {
1209+
// This is the responsibility of LibraryPropertiesDependsFieldInvalidFormat()
1210+
continue
1211+
}
1212+
var dependency semver.Dependency = &librariesindex.Dependency{
1213+
Name: dependencyData[0][1],
1214+
VersionConstraint: dependencyConstraint,
1215+
}
1216+
1217+
logrus.Tracef("Checking if dependency %s is in index.", depend)
1218+
// Get all releases of the dependency
1219+
library := projectdata.LibraryManagerIndex().Index.FindIndexedLibrary(&libraries.Library{Name: dependency.GetName()})
1220+
if library == nil {
1221+
logrus.Tracef("Dependency is not in the index.")
1222+
dependsNotInIndex = append(dependsNotInIndex, depend)
1223+
continue
1224+
}
1225+
// Convert the dependency's libraries.Library object to a semver.Releases object
1226+
var releases semver.Releases
1227+
for _, release := range library.Releases {
1228+
releases = append(releases, release)
1229+
}
1230+
// Filter the dependency's releases according to the dependency's constraint
1231+
dependencyReleases := releases.FilterBy(dependency)
1232+
if len(dependencyReleases) == 0 {
1233+
logrus.Tracef("No releases match dependency's constraint.")
1234+
dependsNotInIndex = append(dependsNotInIndex, depend)
1235+
continue
12001236
}
12011237
}
12021238

1203-
if len(dependenciesNotInIndex) > 0 {
1204-
return ruleresult.Fail, strings.Join(dependenciesNotInIndex, ", ")
1239+
if len(dependsNotInIndex) > 0 {
1240+
return ruleresult.Fail, strings.Join(dependsNotInIndex, ", ")
12051241
}
12061242

12071243
return ruleresult.Pass, ""
@@ -1461,15 +1497,8 @@ func IncorrectExamplesFolderNameCase() (result ruleresult.Type, output string) {
14611497

14621498
// nameInLibraryManagerIndex returns whether there is a library in Library Manager index using the given name.
14631499
func nameInLibraryManagerIndex(name string) bool {
1464-
libraries := projectdata.LibraryManagerIndex()["libraries"].([]interface{})
1465-
for _, libraryInterface := range libraries {
1466-
library := libraryInterface.(map[string]interface{})
1467-
if library["name"].(string) == name {
1468-
return true
1469-
}
1470-
}
1471-
1472-
return false
1500+
library := projectdata.LibraryManagerIndex().Index.FindIndexedLibrary(&libraries.Library{Name: name})
1501+
return library != nil
14731502
}
14741503

14751504
// spellCheckLibraryPropertiesFieldValue returns the value of the provided library.properties field with commonly misspelled words corrected.

‎internal/rule/rulefunction/library_test.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -794,22 +794,25 @@ func TestLibraryPropertiesArchitecturesFieldValueCase(t *testing.T) {
794794
checkLibraryRuleFunction(LibraryPropertiesArchitecturesFieldValueCase, testTables, t)
795795
}
796796

797-
func TestLibraryPropertiesDependsFieldDisallowedCharacters(t *testing.T) {
797+
func TestLibraryPropertiesDependsFieldInvalidFormat(t *testing.T) {
798798
testTables := []libraryRuleFunctionTestTable{
799799
{"Invalid", "InvalidLibraryProperties", ruleresult.NotRun, ""},
800800
{"Legacy", "Legacy", ruleresult.Skip, ""},
801801
{"Depends field has disallowed characters", "DependsHasBadChars", ruleresult.Fail, ""},
802-
{"Valid", "DependsIndexed", ruleresult.Pass, ""},
802+
{"Valid", "DependsValid", ruleresult.Pass, ""},
803803
}
804804

805-
checkLibraryRuleFunction(LibraryPropertiesDependsFieldDisallowedCharacters, testTables, t)
805+
checkLibraryRuleFunction(LibraryPropertiesDependsFieldInvalidFormat, testTables, t)
806806
}
807807

808808
func TestLibraryPropertiesDependsFieldNotInIndex(t *testing.T) {
809809
testTables := []libraryRuleFunctionTestTable{
810810
{"Unable to load", "InvalidLibraryProperties", ruleresult.NotRun, ""},
811+
{"Legacy", "Legacy", ruleresult.Skip, ""},
812+
{"No depends field", "MissingFields", ruleresult.Skip, ""},
811813
{"Dependency not in index", "DependsNotIndexed", ruleresult.Fail, "^NotIndexed$"},
812-
{"Dependency in index", "DependsIndexed", ruleresult.Pass, ""},
814+
{"Dependency constraint not in index", "DependsConstraintNotIndexed", ruleresult.Fail, "^Servo \\(=0\\.0\\.1\\)$"},
815+
{"Dependencies in index", "DependsIndexed", ruleresult.Pass, ""},
813816
{"Depends field empty", "DependsEmpty", ruleresult.Pass, ""},
814817
{"No depends", "NoDepends", ruleresult.Skip, ""},
815818
}

‎internal/rule/rulefunction/testdata/libraries/DependsConstraintNotIndexed/library.properties‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
name=DependsConstraintNotIndexed
2+
version=1.0.0
3+
author=Cristian Maglie <c.maglie@example.com>, Pippo Pluto <pippo@example.com>
4+
maintainer=Cristian Maglie <c.maglie@example.com>
5+
sentence=A library that makes coding a web server a breeze.
6+
paragraph=Supports HTTP1.1 and you can do GET and POST.
7+
category=Communication
8+
url=http://example.com/
9+
architectures=avr
10+
depends=Servo (=0.0.1)
11+
includes=DependsConstraintNotIndexed.h

‎internal/rule/rulefunction/testdata/libraries/DependsConstraintNotIndexed/src/DependsConstraintNotIndexed.h‎

Whitespace-only changes.

‎internal/rule/rulefunction/testdata/libraries/DependsIndexed/library.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,4 @@ paragraph=Supports HTTP1.1 and you can do GET and POST.
77
category=Communication
88
url=http://example.com/
99
architectures=avr
10-
depends=Servo, , Adafruit NeoPixel
10+
depends=,(foo),foo (bar),Adafruit NeoPixel,Servo (<1.1.4),Stepper (<=1.1.3),Mouse (=1.0.0),Keyboard (>=1.0.1),WiFiNINA (>1.0.0)

‎internal/rule/rulefunction/testdata/libraries/DependsValid/library.properties‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
name=DependsValid
2+
version=1.0.0
3+
author=Cristian Maglie <c.maglie@example.com>, Pippo Pluto <pippo@example.com>
4+
maintainer=Cristian Maglie <c.maglie@example.com>
5+
sentence=A library that makes coding a web server a breeze.
6+
paragraph=Supports HTTP1.1 and you can do GET and POST.
7+
category=Communication
8+
url=http://example.com/
9+
architectures=avr
10+
depends=Servo , Adafruit NeoPixel,Stepper (<1.1.3) , Mouse (<=1.0.0),Keyboard (=1.2.3-beta),Ethernet (>=2.0),WiFiNINA (>0.0.8)
11+
includes=DependsValid.h

0 commit comments

Comments
 (0)