From 5d88a4af3d21b9f1c828d94162d43537696910e9 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Mon, 12 Feb 2024 11:09:54 -0800 Subject: [PATCH 01/13] Add parser & checker error handlers to analysis.Config --- tools/analysis/config.go | 4 ++++ tools/analysis/programs.go | 29 +++++++++++++++++++++++++---- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/tools/analysis/config.go b/tools/analysis/config.go index 020384f3ea..707a342654 100644 --- a/tools/analysis/config.go +++ b/tools/analysis/config.go @@ -40,6 +40,10 @@ type Config struct { ) ([]byte, error) // Mode controls the level of information returned for each program Mode LoadMode + // HandleParserError is called when a parser error occurs instead of returning it + HandleParserError func(err ParsingCheckingError) error + // HandleCheckerError is called when a checker error occurs instead of returning it + HandleCheckerError func(err ParsingCheckingError) error } func NewSimpleConfig( diff --git a/tools/analysis/programs.go b/tools/analysis/programs.go index 2aaa20e13a..b549ceb6a0 100644 --- a/tools/analysis/programs.go +++ b/tools/analysis/programs.go @@ -50,7 +50,6 @@ func (programs Programs) load( importRange ast.Range, seenImports importResolutionResults, ) error { - if programs[location] != nil { return nil } @@ -69,14 +68,36 @@ func (programs Programs) load( program, err := parser.ParseProgram(nil, code, parser.Config{}) if err != nil { - return wrapError(err) + // If a parser error handler is set and the error is non-fatal + // use handler to handle the error (e.g. to report it and continue analysis) + // This will only be used for the entry point program (e.g not an imported program) + wrappedErr := wrapError(err) + if config.HandleParserError != nil && program != nil && importingLocation == nil { + err = config.HandleParserError(wrappedErr) + if err != nil { + return err + } + } else { + return wrappedErr + } } var checker *sema.Checker if config.Mode&NeedTypes != 0 { checker, err = programs.check(config, program, location, seenImports) if err != nil { - return wrapError(err) + wrappedErr := wrapError(err) + // If a custom error handler is set, and the error is non-fatal + // use handler to handle the error (e.g. to report it and continue analysis) + // This will only be used for the entry point program (e.g not an imported program) + if config.HandleCheckerError != nil && checker != nil && importingLocation == nil { + err = config.HandleCheckerError(wrappedErr) + if err != nil { + return err + } + } else { + return wrappedErr + } } } @@ -160,7 +181,7 @@ func (programs Programs) check( err = checker.Check() if err != nil { - return nil, err + return checker, err } return checker, nil From 7a717a7c2612b6de15ecafe4f12a2ea0359dd92e Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Mon, 12 Feb 2024 11:25:40 -0800 Subject: [PATCH 02/13] preserve loadError for imports --- tools/analysis/program.go | 9 +++++---- tools/analysis/programs.go | 26 ++++++++++++++++++++++---- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/tools/analysis/program.go b/tools/analysis/program.go index b4612a01e4..9da349093c 100644 --- a/tools/analysis/program.go +++ b/tools/analysis/program.go @@ -27,10 +27,11 @@ import ( ) type Program struct { - Location common.Location - Program *ast.Program - Checker *sema.Checker - Code []byte + Location common.Location + Program *ast.Program + Checker *sema.Checker + Code []byte + loadError error } // Run runs the given DAG of analyzers in parallel diff --git a/tools/analysis/programs.go b/tools/analysis/programs.go index b549ceb6a0..d0c51b6840 100644 --- a/tools/analysis/programs.go +++ b/tools/analysis/programs.go @@ -54,6 +54,7 @@ func (programs Programs) load( return nil } + var loadError error wrapError := func(err error) ParsingCheckingError { return ParsingCheckingError{ error: err, @@ -72,6 +73,10 @@ func (programs Programs) load( // use handler to handle the error (e.g. to report it and continue analysis) // This will only be used for the entry point program (e.g not an imported program) wrappedErr := wrapError(err) + if loadError == nil { + loadError = wrappedErr + } + if config.HandleParserError != nil && program != nil && importingLocation == nil { err = config.HandleParserError(wrappedErr) if err != nil { @@ -87,6 +92,10 @@ func (programs Programs) load( checker, err = programs.check(config, program, location, seenImports) if err != nil { wrappedErr := wrapError(err) + if loadError == nil { + loadError = wrappedErr + } + // If a custom error handler is set, and the error is non-fatal // use handler to handle the error (e.g. to report it and continue analysis) // This will only be used for the entry point program (e.g not an imported program) @@ -102,10 +111,11 @@ func (programs Programs) load( } programs[location] = &Program{ - Location: location, - Code: code, - Program: program, - Checker: checker, + Location: location, + Code: code, + Program: program, + Checker: checker, + loadError: loadError, } return nil @@ -166,6 +176,14 @@ func (programs Programs) check( return nil, err } + // If the imported program has a load error, return it + // This may happen if a program is both imported and the entry point + // and has an error that was handled by a custom error handler + // However, we still want this error in the import resolution + if programs[importedLocation].loadError != nil { + return nil, programs[importedLocation].loadError + } + elaboration = programs[importedLocation].Checker.Elaboration } From b494f95473b5fec6a3572e4b9f41973b4567e7c8 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Tue, 13 Feb 2024 07:29:42 -0800 Subject: [PATCH 03/13] Update tools/analysis/programs.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Müller --- tools/analysis/programs.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/analysis/programs.go b/tools/analysis/programs.go index d0c51b6840..9ed9e83bb3 100644 --- a/tools/analysis/programs.go +++ b/tools/analysis/programs.go @@ -180,8 +180,9 @@ func (programs Programs) check( // This may happen if a program is both imported and the entry point // and has an error that was handled by a custom error handler // However, we still want this error in the import resolution - if programs[importedLocation].loadError != nil { - return nil, programs[importedLocation].loadError + loadError := programs[importedLocation].loadError + if loadError != nil { + return nil, loadError } elaboration = programs[importedLocation].Checker.Elaboration From 61b50212eb0a20d5d83dc319c486221f856041b7 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Tue, 13 Feb 2024 07:33:42 -0800 Subject: [PATCH 04/13] apply suggestions --- tools/analysis/config.go | 5 +++-- tools/analysis/programs.go | 18 ++++++------------ 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/tools/analysis/config.go b/tools/analysis/config.go index 707a342654..f748c1249d 100644 --- a/tools/analysis/config.go +++ b/tools/analysis/config.go @@ -24,6 +24,7 @@ import ( "github.com/onflow/cadence/runtime/ast" "github.com/onflow/cadence/runtime/common" + "github.com/onflow/cadence/runtime/sema" ) // A Config specifies details about how programs should be loaded. @@ -41,9 +42,9 @@ type Config struct { // Mode controls the level of information returned for each program Mode LoadMode // HandleParserError is called when a parser error occurs instead of returning it - HandleParserError func(err ParsingCheckingError) error + HandleParserError func(err ParsingCheckingError, program *ast.Program, importingLocation common.Location) error // HandleCheckerError is called when a checker error occurs instead of returning it - HandleCheckerError func(err ParsingCheckingError) error + HandleCheckerError func(err ParsingCheckingError, checker *sema.Checker, importingLocation common.Location) error } func NewSimpleConfig( diff --git a/tools/analysis/programs.go b/tools/analysis/programs.go index 9ed9e83bb3..23c606e870 100644 --- a/tools/analysis/programs.go +++ b/tools/analysis/programs.go @@ -69,16 +69,12 @@ func (programs Programs) load( program, err := parser.ParseProgram(nil, code, parser.Config{}) if err != nil { - // If a parser error handler is set and the error is non-fatal - // use handler to handle the error (e.g. to report it and continue analysis) - // This will only be used for the entry point program (e.g not an imported program) wrappedErr := wrapError(err) - if loadError == nil { - loadError = wrappedErr - } + loadError = wrappedErr + // If a custom error handler is set, use it to potentially handle the error if config.HandleParserError != nil && program != nil && importingLocation == nil { - err = config.HandleParserError(wrappedErr) + err = config.HandleParserError(wrappedErr, program, importingLocation) if err != nil { return err } @@ -96,11 +92,9 @@ func (programs Programs) load( loadError = wrappedErr } - // If a custom error handler is set, and the error is non-fatal - // use handler to handle the error (e.g. to report it and continue analysis) - // This will only be used for the entry point program (e.g not an imported program) - if config.HandleCheckerError != nil && checker != nil && importingLocation == nil { - err = config.HandleCheckerError(wrappedErr) + // If a custom error handler is set, use it to potentially handle the error + if config.HandleCheckerError != nil { + err = config.HandleCheckerError(wrappedErr, checker, importingLocation) if err != nil { return err } From a2fff5bbb2d85a5e9e1affbb55cd36dbb700c399 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Tue, 13 Feb 2024 07:41:24 -0800 Subject: [PATCH 05/13] Allow program with loadError to be imported --- tools/analysis/config.go | 4 ++-- tools/analysis/programs.go | 29 +++++++++++++---------------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/tools/analysis/config.go b/tools/analysis/config.go index f748c1249d..ffaafd7c93 100644 --- a/tools/analysis/config.go +++ b/tools/analysis/config.go @@ -42,9 +42,9 @@ type Config struct { // Mode controls the level of information returned for each program Mode LoadMode // HandleParserError is called when a parser error occurs instead of returning it - HandleParserError func(err ParsingCheckingError, program *ast.Program, importingLocation common.Location) error + HandleParserError func(err ParsingCheckingError, program *ast.Program) error // HandleCheckerError is called when a checker error occurs instead of returning it - HandleCheckerError func(err ParsingCheckingError, checker *sema.Checker, importingLocation common.Location) error + HandleCheckerError func(err ParsingCheckingError, checker *sema.Checker) error } func NewSimpleConfig( diff --git a/tools/analysis/programs.go b/tools/analysis/programs.go index 23c606e870..65277c2b9a 100644 --- a/tools/analysis/programs.go +++ b/tools/analysis/programs.go @@ -73,8 +73,8 @@ func (programs Programs) load( loadError = wrappedErr // If a custom error handler is set, use it to potentially handle the error - if config.HandleParserError != nil && program != nil && importingLocation == nil { - err = config.HandleParserError(wrappedErr, program, importingLocation) + if config.HandleParserError != nil { + err = config.HandleParserError(wrappedErr, program) if err != nil { return err } @@ -94,7 +94,7 @@ func (programs Programs) load( // If a custom error handler is set, use it to potentially handle the error if config.HandleCheckerError != nil { - err = config.HandleCheckerError(wrappedErr, checker, importingLocation) + err = config.HandleCheckerError(wrappedErr, checker) if err != nil { return err } @@ -149,7 +149,7 @@ func (programs Programs) check( importRange ast.Range, ) (sema.Import, error) { - var elaboration *sema.Elaboration + var imp *sema.ElaborationImport switch importedLocation { case stdlib.CryptoCheckerLocation: cryptoChecker := stdlib.CryptoChecker() @@ -170,21 +170,18 @@ func (programs Programs) check( return nil, err } - // If the imported program has a load error, return it - // This may happen if a program is both imported and the entry point - // and has an error that was handled by a custom error handler - // However, we still want this error in the import resolution - loadError := programs[importedLocation].loadError - if loadError != nil { - return nil, loadError + // If the imported program has a checker, use its elaboration for the import + if programs[importedLocation].Checker != nil { + elaboration := programs[importedLocation].Checker.Elaboration + imp = &sema.ElaborationImport{ + Elaboration: elaboration, + } } - - elaboration = programs[importedLocation].Checker.Elaboration } - return sema.ElaborationImport{ - Elaboration: elaboration, - }, nil + // If the imported program had an error while loading, return it + loadError := programs[importedLocation].loadError + return imp, loadError }, }, ) From 800feb7aa397bb3966048a84daab2198d7a7a386 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Tue, 13 Feb 2024 07:43:33 -0800 Subject: [PATCH 06/13] fixup --- tools/analysis/programs.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tools/analysis/programs.go b/tools/analysis/programs.go index 65277c2b9a..96bc4da15c 100644 --- a/tools/analysis/programs.go +++ b/tools/analysis/programs.go @@ -149,7 +149,9 @@ func (programs Programs) check( importRange ast.Range, ) (sema.Import, error) { - var imp *sema.ElaborationImport + var elaboration *sema.Elaboration + var loadError error + switch importedLocation { case stdlib.CryptoCheckerLocation: cryptoChecker := stdlib.CryptoChecker() @@ -172,16 +174,16 @@ func (programs Programs) check( // If the imported program has a checker, use its elaboration for the import if programs[importedLocation].Checker != nil { - elaboration := programs[importedLocation].Checker.Elaboration - imp = &sema.ElaborationImport{ - Elaboration: elaboration, - } + elaboration = programs[importedLocation].Checker.Elaboration } + + // If the imported program had an error while loading, record it + loadError = programs[importedLocation].loadError } - // If the imported program had an error while loading, return it - loadError := programs[importedLocation].loadError - return imp, loadError + return &sema.ElaborationImport{ + Elaboration: elaboration, + }, loadError }, }, ) From a77323447abe553e84bf5ba96de22f5f03d60217 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Tue, 13 Feb 2024 08:06:16 -0800 Subject: [PATCH 07/13] Refactor and add test --- tools/analysis/analysis_test.go | 68 +++++++++++++++++++++++++++++++++ tools/analysis/program.go | 2 +- tools/analysis/programs.go | 4 +- 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/tools/analysis/analysis_test.go b/tools/analysis/analysis_test.go index 779cf921bf..f916e09d58 100644 --- a/tools/analysis/analysis_test.go +++ b/tools/analysis/analysis_test.go @@ -259,6 +259,74 @@ func TestCheckError(t *testing.T) { require.ErrorAs(t, err, &checkerError) } +// Tests that an error handled by the custom error handler is not returned +// However, it must set LoadError to the handled error so that checkers later importing the program can see it +func TestHandledCheckerErrorImportedProgram(t *testing.T) { + + t.Parallel() + + contract1Address := common.MustBytesToAddress([]byte{0x1}) + contract1Location := common.AddressLocation{ + Address: contract1Address, + Name: "ContractA", + } + const contract1Code = ` + import ContractB from 0x2 + + access(all) contract ContractA { + init() {} + } + ` + contract2Address := common.MustBytesToAddress([]byte{0x2}) + contract2Location := common.AddressLocation{ + Address: contract2Address, + Name: "ContractB", + } + const contract2Code = ` + access(all) contract ContractA { + init() { + X + } + } + ` + + config := &analysis.Config{ + Mode: analysis.NeedTypes, + ResolveCode: func( + location common.Location, + importingLocation common.Location, + importRange ast.Range, + ) ([]byte, error) { + switch location { + case contract1Location: + return []byte(contract1Code), nil + case contract2Location: + return []byte(contract2Code), nil + default: + require.FailNow(t, + "import of unknown location: %s", + "location: %s", + location, + ) + return nil, nil + } + }, + HandleCheckerError: func(err analysis.ParsingCheckingError, _ *sema.Checker) error { + return nil + }, + } + + programs, err := analysis.Load(config, contract1Location) + require.NoError(t, err) + + require.Error(t, programs[contract1Location].LoadError) + require.Error(t, programs[contract2Location].LoadError) + + var checkerError *sema.CheckerError + require.ErrorAs(t, programs[contract1Location].LoadError, &checkerError) + require.ErrorAs(t, programs[contract2Location].LoadError, &checkerError) +} + func TestStdlib(t *testing.T) { t.Parallel() diff --git a/tools/analysis/program.go b/tools/analysis/program.go index 9da349093c..d234adf74c 100644 --- a/tools/analysis/program.go +++ b/tools/analysis/program.go @@ -31,7 +31,7 @@ type Program struct { Program *ast.Program Checker *sema.Checker Code []byte - loadError error + LoadError error } // Run runs the given DAG of analyzers in parallel diff --git a/tools/analysis/programs.go b/tools/analysis/programs.go index 96bc4da15c..530cc8f082 100644 --- a/tools/analysis/programs.go +++ b/tools/analysis/programs.go @@ -109,7 +109,7 @@ func (programs Programs) load( Code: code, Program: program, Checker: checker, - loadError: loadError, + LoadError: loadError, } return nil @@ -178,7 +178,7 @@ func (programs Programs) check( } // If the imported program had an error while loading, record it - loadError = programs[importedLocation].loadError + loadError = programs[importedLocation].LoadError } return &sema.ElaborationImport{ From 096ba33767b08a823891e3ddeddface8499992f6 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Tue, 13 Feb 2024 08:09:03 -0800 Subject: [PATCH 08/13] add test --- tools/analysis/analysis_test.go | 102 ++++++++++++++++++++++++++++++-- 1 file changed, 98 insertions(+), 4 deletions(-) diff --git a/tools/analysis/analysis_test.go b/tools/analysis/analysis_test.go index f916e09d58..57bd7bc27f 100644 --- a/tools/analysis/analysis_test.go +++ b/tools/analysis/analysis_test.go @@ -259,9 +259,106 @@ func TestCheckError(t *testing.T) { require.ErrorAs(t, err, &checkerError) } +func TestHandledParserError(t *testing.T) { + + t.Parallel() + + contractAddress := common.MustBytesToAddress([]byte{0x1}) + contractLocation := common.AddressLocation{ + Address: contractAddress, + Name: "ContractA", + } + const contractCode = ` + access(all) contract ContractA { + init() { + ??? + } + } + ` + + config := &analysis.Config{ + Mode: analysis.NeedSyntax, + ResolveCode: func( + location common.Location, + importingLocation common.Location, + importRange ast.Range, + ) ([]byte, error) { + switch location { + case contractLocation: + return []byte(contractCode), nil + + default: + require.FailNow(t, + "import of unknown location: %s", + "location: %s", + location, + ) + return nil, nil + } + }, + HandleParserError: func(err analysis.ParsingCheckingError, _ *ast.Program) error { + return nil + }, + } + + programs, err := analysis.Load(config, contractLocation) + require.NoError(t, err) + + var parserError *parser.Error + require.ErrorAs(t, programs[contractLocation].LoadError, &parserError) +} + +func TestHandledCheckerError(t *testing.T) { + + t.Parallel() + + contractAddress := common.MustBytesToAddress([]byte{0x1}) + contractLocation := common.AddressLocation{ + Address: contractAddress, + Name: "ContractA", + } + const contractCode = ` + access(all) contract ContractA { + init() { + X + } + } + ` + + config := &analysis.Config{ + Mode: analysis.NeedTypes, + ResolveCode: func( + location common.Location, + importingLocation common.Location, + importRange ast.Range, + ) ([]byte, error) { + switch location { + case contractLocation: + return []byte(contractCode), nil + default: + require.FailNow(t, + "import of unknown location: %s", + "location: %s", + location, + ) + return nil, nil + } + }, + HandleCheckerError: func(err analysis.ParsingCheckingError, _ *sema.Checker) error { + return nil + }, + } + + programs, err := analysis.Load(config, contractLocation) + require.NoError(t, err) + + var checkerError *sema.CheckerError + require.ErrorAs(t, programs[contractLocation].LoadError, &checkerError) +} + // Tests that an error handled by the custom error handler is not returned // However, it must set LoadError to the handled error so that checkers later importing the program can see it -func TestHandledCheckerErrorImportedProgram(t *testing.T) { +func TestHandledLoadErrorImportedProgram(t *testing.T) { t.Parallel() @@ -319,9 +416,6 @@ func TestHandledCheckerErrorImportedProgram(t *testing.T) { programs, err := analysis.Load(config, contract1Location) require.NoError(t, err) - require.Error(t, programs[contract1Location].LoadError) - require.Error(t, programs[contract2Location].LoadError) - var checkerError *sema.CheckerError require.ErrorAs(t, programs[contract1Location].LoadError, &checkerError) require.ErrorAs(t, programs[contract2Location].LoadError, &checkerError) From 2a486006e9fa51187fe60e286c8433ad0b2f9124 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Tue, 13 Feb 2024 08:40:59 -0800 Subject: [PATCH 09/13] improve test --- tools/analysis/analysis_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tools/analysis/analysis_test.go b/tools/analysis/analysis_test.go index 57bd7bc27f..b96765b48c 100644 --- a/tools/analysis/analysis_test.go +++ b/tools/analysis/analysis_test.go @@ -380,7 +380,7 @@ func TestHandledLoadErrorImportedProgram(t *testing.T) { Name: "ContractB", } const contract2Code = ` - access(all) contract ContractA { + access(all) contract ContractB { init() { X } @@ -419,6 +419,13 @@ func TestHandledLoadErrorImportedProgram(t *testing.T) { var checkerError *sema.CheckerError require.ErrorAs(t, programs[contract1Location].LoadError, &checkerError) require.ErrorAs(t, programs[contract2Location].LoadError, &checkerError) + + loadErr := programs[contract1Location].LoadError.(analysis.ParsingCheckingError) + unwrapedErr := loadErr.Unwrap().(*sema.CheckerError) + + var importedProgramErr *sema.ImportedProgramError + require.Len(t, unwrapedErr.ChildErrors(), 1) + require.ErrorAs(t, unwrapedErr.ChildErrors()[0], &importedProgramErr) } func TestStdlib(t *testing.T) { From c0abed8a74727116514b5e4ba059c088ece4d36e Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Tue, 13 Feb 2024 08:42:27 -0800 Subject: [PATCH 10/13] add comment --- tools/analysis/analysis_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/analysis/analysis_test.go b/tools/analysis/analysis_test.go index b96765b48c..cbaa167ed0 100644 --- a/tools/analysis/analysis_test.go +++ b/tools/analysis/analysis_test.go @@ -420,10 +420,10 @@ func TestHandledLoadErrorImportedProgram(t *testing.T) { require.ErrorAs(t, programs[contract1Location].LoadError, &checkerError) require.ErrorAs(t, programs[contract2Location].LoadError, &checkerError) + // Validate that parent checker receives the imported program error despite it being handled + var importedProgramErr *sema.ImportedProgramError loadErr := programs[contract1Location].LoadError.(analysis.ParsingCheckingError) unwrapedErr := loadErr.Unwrap().(*sema.CheckerError) - - var importedProgramErr *sema.ImportedProgramError require.Len(t, unwrapedErr.ChildErrors(), 1) require.ErrorAs(t, unwrapedErr.ChildErrors()[0], &importedProgramErr) } From ad8191ec1d6d1ceb08c3c2a9ef17df68fe3b0cbc Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Tue, 13 Feb 2024 09:01:06 -0800 Subject: [PATCH 11/13] record handler calls & switch failnow to failnowf --- tools/analysis/analysis_test.go | 50 ++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/tools/analysis/analysis_test.go b/tools/analysis/analysis_test.go index cbaa167ed0..514c5b5129 100644 --- a/tools/analysis/analysis_test.go +++ b/tools/analysis/analysis_test.go @@ -76,8 +76,8 @@ func TestNeedSyntaxAndImport(t *testing.T) { return []byte(contractCode), nil default: - require.FailNow(t, - "import of unknown location: %s", + require.FailNowf(t, + "import of unknown location", "location: %s", location, ) @@ -192,12 +192,9 @@ func TestParseError(t *testing.T) { importRange ast.Range, ) ([]byte, error) { switch location { - case contractLocation: - return []byte(contractCode), nil - default: - require.FailNow(t, - "import of unknown location: %s", + require.FailNowf(t, + "import of unknown location", "location: %s", location, ) @@ -242,8 +239,8 @@ func TestCheckError(t *testing.T) { return []byte(contractCode), nil default: - require.FailNow(t, - "import of unknown location: %s", + require.FailNowf(t, + "import of unknown location", "location: %s", location, ) @@ -276,6 +273,7 @@ func TestHandledParserError(t *testing.T) { } ` + handlerCalls := 0 config := &analysis.Config{ Mode: analysis.NeedSyntax, ResolveCode: func( @@ -288,8 +286,8 @@ func TestHandledParserError(t *testing.T) { return []byte(contractCode), nil default: - require.FailNow(t, - "import of unknown location: %s", + require.FailNowf(t, + "import of unknown location", "location: %s", location, ) @@ -297,10 +295,14 @@ func TestHandledParserError(t *testing.T) { } }, HandleParserError: func(err analysis.ParsingCheckingError, _ *ast.Program) error { + require.Error(t, err) + handlerCalls++ return nil }, } + require.Equal(t, 1, handlerCalls) + programs, err := analysis.Load(config, contractLocation) require.NoError(t, err) @@ -325,6 +327,7 @@ func TestHandledCheckerError(t *testing.T) { } ` + handlerCalls := 0 config := &analysis.Config{ Mode: analysis.NeedTypes, ResolveCode: func( @@ -336,8 +339,8 @@ func TestHandledCheckerError(t *testing.T) { case contractLocation: return []byte(contractCode), nil default: - require.FailNow(t, - "import of unknown location: %s", + require.FailNowf(t, + "import of unknown location", "location: %s", location, ) @@ -345,10 +348,14 @@ func TestHandledCheckerError(t *testing.T) { } }, HandleCheckerError: func(err analysis.ParsingCheckingError, _ *sema.Checker) error { + require.Error(t, err) + handlerCalls++ return nil }, } + require.Equal(t, 1, handlerCalls) + programs, err := analysis.Load(config, contractLocation) require.NoError(t, err) @@ -387,6 +394,7 @@ func TestHandledLoadErrorImportedProgram(t *testing.T) { } ` + handlerCalls := 0 config := &analysis.Config{ Mode: analysis.NeedTypes, ResolveCode: func( @@ -400,8 +408,8 @@ func TestHandledLoadErrorImportedProgram(t *testing.T) { case contract2Location: return []byte(contract2Code), nil default: - require.FailNow(t, - "import of unknown location: %s", + require.FailNowf(t, + "import of unknown location", "location: %s", location, ) @@ -409,10 +417,14 @@ func TestHandledLoadErrorImportedProgram(t *testing.T) { } }, HandleCheckerError: func(err analysis.ParsingCheckingError, _ *sema.Checker) error { + require.Error(t, err) + handlerCalls++ return nil }, } + require.Equal(t, 2, handlerCalls) + programs, err := analysis.Load(config, contract1Location) require.NoError(t, err) @@ -452,8 +464,8 @@ func TestStdlib(t *testing.T) { return []byte(code), nil default: - require.FailNow(t, - "import of unknown location: %s", + require.FailNowf(t, + "import of unknown location", "location: %s", location, ) @@ -518,8 +530,8 @@ func TestCyclicImports(t *testing.T) { return []byte(barContractCode), nil default: - require.FailNow(t, - "import of unknown location: %s", + require.FailNowf(t, + "import of unknown location", "location: %s", location, ) From 2497d4c41da10372e0dcadca650be53a0f7c2eb5 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Tue, 13 Feb 2024 09:04:45 -0800 Subject: [PATCH 12/13] fix subchecker return value --- tools/analysis/programs.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/analysis/programs.go b/tools/analysis/programs.go index 530cc8f082..08801208f3 100644 --- a/tools/analysis/programs.go +++ b/tools/analysis/programs.go @@ -181,9 +181,13 @@ func (programs Programs) check( loadError = programs[importedLocation].LoadError } - return &sema.ElaborationImport{ + if loadError != nil { + return nil, loadError + } + + return sema.ElaborationImport{ Elaboration: elaboration, - }, loadError + }, nil }, }, ) From b714e998ccd620c0660e357e116f72676514d5e1 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Tue, 13 Feb 2024 10:13:40 -0800 Subject: [PATCH 13/13] address feedback --- tools/analysis/analysis_test.go | 19 ++++++++----------- tools/analysis/programs.go | 9 ++++++--- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tools/analysis/analysis_test.go b/tools/analysis/analysis_test.go index 514c5b5129..fb5d885d05 100644 --- a/tools/analysis/analysis_test.go +++ b/tools/analysis/analysis_test.go @@ -192,6 +192,8 @@ func TestParseError(t *testing.T) { importRange ast.Range, ) ([]byte, error) { switch location { + case contractLocation: + return []byte(contractCode), nil default: require.FailNowf(t, "import of unknown location", @@ -301,12 +303,12 @@ func TestHandledParserError(t *testing.T) { }, } - require.Equal(t, 1, handlerCalls) - programs, err := analysis.Load(config, contractLocation) require.NoError(t, err) - var parserError *parser.Error + require.Equal(t, 1, handlerCalls) + + var parserError parser.Error require.ErrorAs(t, programs[contractLocation].LoadError, &parserError) } @@ -354,9 +356,8 @@ func TestHandledCheckerError(t *testing.T) { }, } - require.Equal(t, 1, handlerCalls) - programs, err := analysis.Load(config, contractLocation) + require.Equal(t, 1, handlerCalls) require.NoError(t, err) var checkerError *sema.CheckerError @@ -423,9 +424,8 @@ func TestHandledLoadErrorImportedProgram(t *testing.T) { }, } - require.Equal(t, 2, handlerCalls) - programs, err := analysis.Load(config, contract1Location) + require.Equal(t, 2, handlerCalls) require.NoError(t, err) var checkerError *sema.CheckerError @@ -434,10 +434,7 @@ func TestHandledLoadErrorImportedProgram(t *testing.T) { // Validate that parent checker receives the imported program error despite it being handled var importedProgramErr *sema.ImportedProgramError - loadErr := programs[contract1Location].LoadError.(analysis.ParsingCheckingError) - unwrapedErr := loadErr.Unwrap().(*sema.CheckerError) - require.Len(t, unwrapedErr.ChildErrors(), 1) - require.ErrorAs(t, unwrapedErr.ChildErrors()[0], &importedProgramErr) + require.ErrorAs(t, programs[contract1Location].LoadError, &importedProgramErr) } func TestStdlib(t *testing.T) { diff --git a/tools/analysis/programs.go b/tools/analysis/programs.go index 08801208f3..767dea6465 100644 --- a/tools/analysis/programs.go +++ b/tools/analysis/programs.go @@ -172,13 +172,16 @@ func (programs Programs) check( return nil, err } + program := programs[importedLocation] + checker := program.Checker + // If the imported program has a checker, use its elaboration for the import - if programs[importedLocation].Checker != nil { - elaboration = programs[importedLocation].Checker.Elaboration + if checker != nil { + elaboration = checker.Elaboration } // If the imported program had an error while loading, record it - loadError = programs[importedLocation].LoadError + loadError = program.LoadError } if loadError != nil {