From 0c3b3588175f4f74e1112298e14348668f4e5fcf Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 10 Jun 2021 15:21:31 -0700 Subject: [PATCH 1/2] Make package index loading function handle missing file Each possible problem with the project is handled by a dedicated rule. One such problem is that the project was not found. Each rule must be configured so that unrelated problems with the project will not cause them to break or provide incorrect results. This can result in a lot of redundant boilerplate code. Before running a rule against the package index data, it is always necessary to check whether the data is available. If not, running the rule is abandoned with a "NotRun" result. This which results in just such boilerplate, but is likely to be unavoidable without breaking from the modular architecture of the rules system. If the package index file itself was not found, then the loading of the data can not be accomplished, and thus a missing package index should also be cause for abandoning the run of data rules. The check for data can serve as an implicit check for the existence of the data source, meaning additional rule function boilerplate for file existence check can be avoided. --- internal/project/packageindex/packageindex.go | 5 ++++- internal/project/packageindex/packageindex_test.go | 10 +++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/internal/project/packageindex/packageindex.go b/internal/project/packageindex/packageindex.go index 1d0560df..a96d0bdc 100644 --- a/internal/project/packageindex/packageindex.go +++ b/internal/project/packageindex/packageindex.go @@ -32,9 +32,12 @@ import ( // Properties parses the package index from the given path and returns the data. func Properties(packageIndexPath *paths.Path) (map[string]interface{}, error) { + if packageIndexPath == nil { + return nil, fmt.Errorf("Package index path is nil") + } rawIndex, err := packageIndexPath.ReadFile() if err != nil { - panic(err) + return nil, err } var indexData map[string]interface{} err = json.Unmarshal(rawIndex, &indexData) diff --git a/internal/project/packageindex/packageindex_test.go b/internal/project/packageindex/packageindex_test.go index 8d8375e1..5fdadd28 100644 --- a/internal/project/packageindex/packageindex_test.go +++ b/internal/project/packageindex/packageindex_test.go @@ -35,10 +35,14 @@ func init() { } func TestProperties(t *testing.T) { - packageIndex, err := Properties(testDataPath.Join("package_valid_index.json")) - require.Nil(t, err) + assert.NotPanics(t, func() { Properties(nil) }, "Don't panic when package index was not found") + packageIndex, err := Properties(nil) + assert.Error(t, err, "Error when package index was not found") - assert.NotNil(t, packageIndex) + packageIndex, err = Properties(testDataPath.Join("package_valid_index.json")) + require.Nil(t, err, "No error on valid package index") + + assert.NotNil(t, packageIndex, "Package index data present") } func TestHasValidExtension(t *testing.T) { From 447d7cd363e243fb714b321490249d9a6e3071bf Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 10 Jun 2021 15:50:07 -0700 Subject: [PATCH 2/2] Provide natively loaded data as package index project data Arduino CLI's package index loading function can can be useful in its current capacity as a general check on the validity of the package index as far as Arduino CLI is concerned, used for that purpose by a single dedicated rule. However, it can not be used as a data source for other rules because Arduino CLI should attempt to be resilient to bad data when possible, while Arduino Lint needs to detect the very bad data that Arduino CLI might correct. So the data from the native loading code must be used in all rules, while the value from Arduino CLI's load is solely in whether or not it returned an error. --- internal/project/projectdata/packageindex.go | 21 +++++++++++++------ .../project/projectdata/packageindex_test.go | 16 +++++++------- internal/rule/rulefunction/packageindex.go | 4 ++-- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/internal/project/projectdata/packageindex.go b/internal/project/projectdata/packageindex.go index a0f8f679..4cd65bb0 100644 --- a/internal/project/projectdata/packageindex.go +++ b/internal/project/projectdata/packageindex.go @@ -16,26 +16,35 @@ package projectdata import ( - "github.com/arduino/arduino-cli/arduino/cores/packageindex" + clipackageindex "github.com/arduino/arduino-cli/arduino/cores/packageindex" + "github.com/arduino/arduino-lint/internal/project/packageindex" ) // InitializeForPackageIndex gathers the package index rule data for the specified project. func InitializeForPackageIndex() { + packageIndex, packageIndexLoadError = packageindex.Properties(ProjectPath()) if ProjectPath() != nil { - packageIndex, packageIndexLoadError = packageindex.LoadIndex(ProjectPath()) + _, packageIndexCLILoadError = clipackageindex.LoadIndex(ProjectPath()) } } -var packageIndex *packageindex.Index +var packageIndex map[string]interface{} -// PackageIndex returns the packageindex.Index object generated by Arduino CLI. -func PackageIndex() *packageindex.Index { +// PackageIndex returns the package index data. +func PackageIndex() map[string]interface{} { return packageIndex } var packageIndexLoadError error -// PackageIndexLoadError returns the error return of packageindex.LoadIndex(). +// PackageIndexLoadError returns the error from loading the package index. func PackageIndexLoadError() error { return packageIndexLoadError } + +var packageIndexCLILoadError error + +// PackageIndexCLILoadError returns the error return of Arduino CLI's packageindex.LoadIndex(). +func PackageIndexCLILoadError() error { + return packageIndexCLILoadError +} diff --git a/internal/project/projectdata/packageindex_test.go b/internal/project/projectdata/packageindex_test.go index 800b01e0..372381c0 100644 --- a/internal/project/projectdata/packageindex_test.go +++ b/internal/project/projectdata/packageindex_test.go @@ -36,14 +36,15 @@ func init() { func TestInitializeForPackageIndex(t *testing.T) { testTables := []struct { - testName string - path *paths.Path - packageIndexAssertion assert.ValueAssertionFunc - packageIndexLoadErrorAssertion assert.ValueAssertionFunc + testName string + path *paths.Path + packageIndexAssertion assert.ValueAssertionFunc + packageIndexLoadErrorAssertion assert.ValueAssertionFunc + packageIndexCLILoadErrorAssertion assert.ValueAssertionFunc }{ - {"Valid", packageIndexTestDataPath.Join("valid-package-index", "package_foo_index.json"), assert.NotNil, assert.Nil}, - {"Invalid package index", packageIndexTestDataPath.Join("invalid-package-index", "package_foo_index.json"), assert.Nil, assert.NotNil}, - {"Invalid JSON", packageIndexTestDataPath.Join("invalid-JSON", "package_foo_index.json"), assert.Nil, assert.NotNil}, + {"Valid", packageIndexTestDataPath.Join("valid-package-index", "package_foo_index.json"), assert.NotNil, assert.Nil, assert.Nil}, + {"Invalid package index", packageIndexTestDataPath.Join("invalid-package-index", "package_foo_index.json"), assert.Nil, assert.NotNil, assert.NotNil}, + {"Invalid JSON", packageIndexTestDataPath.Join("invalid-JSON", "package_foo_index.json"), assert.Nil, assert.NotNil, assert.NotNil}, } for _, testTable := range testTables { @@ -56,6 +57,7 @@ func TestInitializeForPackageIndex(t *testing.T) { Initialize(testProject) testTable.packageIndexLoadErrorAssertion(t, PackageIndexLoadError(), testTable.testName) + testTable.packageIndexCLILoadErrorAssertion(t, PackageIndexCLILoadError(), testTable.testName) if PackageIndexLoadError() == nil { testTable.packageIndexAssertion(t, PackageIndex(), testTable.testName) } diff --git a/internal/rule/rulefunction/packageindex.go b/internal/rule/rulefunction/packageindex.go index a112c012..7a82eb74 100644 --- a/internal/rule/rulefunction/packageindex.go +++ b/internal/rule/rulefunction/packageindex.go @@ -77,8 +77,8 @@ func PackageIndexFormat() (result ruleresult.Type, output string) { return ruleresult.NotRun, "Package index not found" } - if projectdata.PackageIndexLoadError() != nil { - return ruleresult.Fail, projectdata.PackageIndexLoadError().Error() + if projectdata.PackageIndexCLILoadError() != nil { + return ruleresult.Fail, projectdata.PackageIndexCLILoadError().Error() } return ruleresult.Pass, ""