From bbb37db5bca8780e885bb10f2da56ac41f6592fb Mon Sep 17 00:00:00 2001 From: per1234 Date: Mon, 23 Nov 2020 04:51:14 -0800 Subject: [PATCH 1/3] Use properties.LoadFromPath() to load library.properties Since the properties package has a function supporting the paths.Path type the library.properties path starts in, it was silly to convert it to a string for use with properties.Load(). --- project/library/libraryproperties/libraryproperties.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/library/libraryproperties/libraryproperties.go b/project/library/libraryproperties/libraryproperties.go index cf7bb55b7..de7184fb3 100644 --- a/project/library/libraryproperties/libraryproperties.go +++ b/project/library/libraryproperties/libraryproperties.go @@ -26,7 +26,7 @@ import ( // Properties parses the library.properties from the given path and returns the data. func Properties(libraryPath *paths.Path) (*properties.Map, error) { - libraryProperties, err := properties.Load(libraryPath.Join("library.properties").String()) + libraryProperties, err := properties.LoadFromPath(libraryPath.Join("library.properties")) if err != nil { return nil, err } From 3209322f7fb64c1eed7790ddbed110b6dd026b71 Mon Sep 17 00:00:00 2001 From: per1234 Date: Tue, 1 Dec 2020 09:24:36 -0800 Subject: [PATCH 2/3] Use properties.SafeLoadFromPath() to load library.properties In the event of a problem loading, this will return an empty map rather than nil. The error already serves as the indicator of this problem for the invalid library.properties check, so having an empty map as the output only simplifies the handling of that situation in the other checks. --- project/library/libraryproperties/libraryproperties.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/project/library/libraryproperties/libraryproperties.go b/project/library/libraryproperties/libraryproperties.go index de7184fb3..ba2837c93 100644 --- a/project/library/libraryproperties/libraryproperties.go +++ b/project/library/libraryproperties/libraryproperties.go @@ -26,11 +26,7 @@ import ( // Properties parses the library.properties from the given path and returns the data. func Properties(libraryPath *paths.Path) (*properties.Map, error) { - libraryProperties, err := properties.LoadFromPath(libraryPath.Join("library.properties")) - if err != nil { - return nil, err - } - return libraryProperties, nil + return properties.SafeLoadFromPath(libraryPath.Join("library.properties")) } var schemaObject = make(map[compliancelevel.Type]*jsonschema.Schema) From 98d801160b2921f5f9cc44e3f2cb041d32a833bd Mon Sep 17 00:00:00 2001 From: per1234 Date: Tue, 1 Dec 2020 10:32:02 -0800 Subject: [PATCH 3/3] Handle libraries.Load() failure The issue that caused the failure will be caught by a specific check, so the other checks that use the data returned by this function should simply not run. --- check/checkfunctions/library.go | 19 ++++++++++++------- check/checkfunctions/library_test.go | 4 ++++ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/check/checkfunctions/library.go b/check/checkfunctions/library.go index 82fdc95f5..1b91d315a 100644 --- a/check/checkfunctions/library.go +++ b/check/checkfunctions/library.go @@ -38,14 +38,23 @@ import ( // LibraryPropertiesFormat checks for invalid library.properties format. func LibraryPropertiesFormat() (result checkresult.Type, output string) { + if checkdata.LoadedLibrary() != nil && checkdata.LoadedLibrary().IsLegacy { + return checkresult.NotRun, "" + } + if checkdata.LibraryPropertiesLoadError() != nil { return checkresult.Fail, checkdata.LibraryPropertiesLoadError().Error() } + return checkresult.Pass, "" } // LibraryPropertiesMissing checks for presence of library.properties. func LibraryPropertiesMissing() (result checkresult.Type, output string) { + if checkdata.LoadedLibrary() == nil { + return checkresult.NotRun, "" + } + if checkdata.LoadedLibrary().IsLegacy { return checkresult.Fail, "" } @@ -768,11 +777,7 @@ func LibraryPropertiesDotALinkageFieldInvalid() (result checkresult.Type, output // LibraryPropertiesDotALinkageFieldTrueWithFlatLayout checks whether a library using the "dot_a_linkage" feature has the required recursive layout type. func LibraryPropertiesDotALinkageFieldTrueWithFlatLayout() (result checkresult.Type, output string) { - if checkdata.LoadedLibrary() == nil { - return checkresult.NotRun, "" - } - - if !checkdata.LibraryProperties().ContainsKey("dot_a_linkage") { + if checkdata.LoadedLibrary() == nil || !checkdata.LibraryProperties().ContainsKey("dot_a_linkage") { return checkresult.NotRun, "" } @@ -905,7 +910,7 @@ func LibraryPropertiesMisspelledOptionalField() (result checkresult.Type, output // LibraryInvalid checks whether the provided path is a valid library. func LibraryInvalid() (result checkresult.Type, output string) { - if library.ContainsHeaderFile(checkdata.LoadedLibrary().SourceDir) { + if checkdata.LoadedLibrary() != nil && library.ContainsHeaderFile(checkdata.LoadedLibrary().SourceDir) { return checkresult.Pass, "" } @@ -1120,7 +1125,7 @@ func MisspelledExtrasFolderName() (result checkresult.Type, output string) { // RecursiveLibraryWithUtilityFolder checks for presence of a `utility` subfolder in a recursive layout library. func RecursiveLibraryWithUtilityFolder() (result checkresult.Type, output string) { - if checkdata.LoadedLibrary().Layout == libraries.FlatLayout { + if checkdata.LoadedLibrary() == nil || checkdata.LoadedLibrary().Layout == libraries.FlatLayout { return checkresult.NotRun, "" } diff --git a/check/checkfunctions/library_test.go b/check/checkfunctions/library_test.go index 3d6c0480f..af81e6218 100644 --- a/check/checkfunctions/library_test.go +++ b/check/checkfunctions/library_test.go @@ -84,6 +84,7 @@ func TestIncorrectLibraryPropertiesFileNameCase(t *testing.T) { func TestLibraryPropertiesMissing(t *testing.T) { testTables := []libraryCheckFunctionTestTable{ + {"Invalid non-legacy", "InvalidLibraryProperties", checkresult.NotRun, ""}, {"Legacy", "Legacy", checkresult.Fail, ""}, {"Flat non-legacy", "Flat", checkresult.Pass, ""}, {"Recursive", "Recursive", checkresult.Pass, ""}, @@ -104,6 +105,7 @@ func TestRedundantLibraryProperties(t *testing.T) { func TestLibraryPropertiesFormat(t *testing.T) { testTables := []libraryCheckFunctionTestTable{ {"Invalid", "InvalidLibraryProperties", checkresult.Fail, ""}, + {"Legacy", "Legacy", checkresult.NotRun, ""}, {"Valid", "Recursive", checkresult.Pass, ""}, } @@ -242,6 +244,7 @@ func TestLibraryPropertiesPrecompiledFieldEnabledWithFlatLayout(t *testing.T) { func TestLibraryInvalid(t *testing.T) { testTables := []libraryCheckFunctionTestTable{ + {"Invalid library.properties", "InvalidLibraryProperties", checkresult.Fail, ""}, {"Invalid flat layout", "FlatWithoutHeader", checkresult.Fail, ""}, {"Invalid recursive layout", "RecursiveWithoutLibraryProperties", checkresult.Fail, ""}, {"Valid library", "Recursive", checkresult.Pass, ""}, @@ -383,6 +386,7 @@ func TestIncorrectExtrasFolderNameCase(t *testing.T) { func TestRecursiveLibraryWithUtilityFolder(t *testing.T) { testTables := []libraryCheckFunctionTestTable{ + {"Unable to load", "InvalidLibraryProperties", checkresult.NotRun, ""}, {"Flat", "Flat", checkresult.NotRun, ""}, {"Recursive with utility", "RecursiveWithUtilityFolder", checkresult.Fail, ""}, {"Recursive without utility", "Recursive", checkresult.Pass, ""},