Prevent cyclic imports

runtime/cmd/cmd.go

@@ -95,7 +95,7 @@ func PrepareChecker(
 		sema.WithPredeclaredValues(valueDeclarations.ToSemaValueDeclarations()),
 		sema.WithPredeclaredTypes(typeDeclarations),
 		sema.WithImportHandler(
-			func(checker *sema.Checker, importedLocation common.Location) (sema.Import, error) {
+			func(checker *sema.Checker, importedLocation common.Location, importRange ast.Range) (sema.Import, error) {
 				stringLocation, ok := importedLocation.(common.StringLocation)
 
 				if !ok {

runtime/import_test.go

@@ -0,0 +1,115 @@
+/*
+ * Cadence - The resource-oriented smart contract programming language
+ *
+ * Copyright 2019-2020 Dapper Labs, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package runtime
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/onflow/cadence/runtime/common"
+	"github.com/onflow/cadence/runtime/sema"
+	"github.com/onflow/cadence/runtime/tests/checker"
+)
+
+func TestRuntimeCyclicImport(t *testing.T) {
+
+	t.Parallel()
+
+	runtime := NewInterpreterRuntime()
+
+	imported1 := []byte(`
+      import p2
+    `)
+
+	imported2 := []byte(`
+      import p1
+    `)
+
+	script := []byte(`
+      import p1
+
+      pub fun main() {}
+    `)
+
+	var checkCount int
+
+	runtimeInterface := &testRuntimeInterface{
+		getCode: func(location Location) (bytes []byte, err error) {
+			switch location {
+			case common.IdentifierLocation("p1"):
+				return imported1, nil
+			case common.IdentifierLocation("p2"):
+				return imported2, nil
+			default:
+				return nil, fmt.Errorf("unknown import location: %s", location)
+			}
+		},
+		programChecked: func(location common.Location, duration time.Duration) {
+			checkCount += 1
+		},
+	}
+
+	nextTransactionLocation := newTransactionLocationGenerator()
+
+	location := nextTransactionLocation()
+	_, err := runtime.ExecuteScript(
+		Script{
+			Source: script,
+		},
+		Context{
+			Interface: runtimeInterface,
+			Location:  location,
+		},
+	)
+	require.Error(t, err)
+
+	require.Contains(t, err.Error(), "cyclic import of `p1`")
+
+	// Script
+
+	var checkerErr *sema.CheckerError
+	require.ErrorAs(t, err, &checkerErr)
+
+	errs := checker.ExpectCheckerErrors(t, checkerErr, 1)
+
+	var importedProgramErr *sema.ImportedProgramError
+	require.ErrorAs(t, errs[0], &importedProgramErr)
+
+	// P1
+
+	var checkerErr2 *sema.CheckerError
+	require.ErrorAs(t, importedProgramErr.Err, &checkerErr2)
+
+	errs = checker.ExpectCheckerErrors(t, checkerErr2, 1)
+
+	var importedProgramErr2 *sema.ImportedProgramError
+	require.ErrorAs(t, errs[0], &importedProgramErr2)
+
+	// P2
+
+	var checkerErr3 *sema.CheckerError
+	require.ErrorAs(t, importedProgramErr2.Err, &checkerErr3)
+
+	errs = checker.ExpectCheckerErrors(t, checkerErr3, 1)
+
+	require.IsType(t, &sema.CyclicImportsError{}, errs[0])
+}

runtime/repl.go

@@ -62,7 +62,7 @@ func NewREPL(
 			sema.WithPredeclaredTypes(typeDeclarations),
 			sema.WithAccessCheckMode(sema.AccessCheckModeNotSpecifiedUnrestricted),
 			sema.WithImportHandler(
-				func(checker *sema.Checker, importedLocation common.Location) (sema.Import, error) {
+				func(checker *sema.Checker, importedLocation common.Location, importRange ast.Range) (sema.Import, error) {
 					stringLocation, ok := importedLocation.(common.StringLocation)
 
 					if !ok {

runtime/runtime.go

@@ -42,6 +42,8 @@ type Script struct {
 	Arguments [][]byte
 }
 
+type importResolutionResults map[common.LocationID]bool
+
 // Runtime is a runtime capable of executing Cadence.
 type Runtime interface {
 	// ExecuteScript executes the given script.
@@ -176,6 +178,7 @@ func (r *interpreterRuntime) ExecuteScript(script Script, context Context) (cade
 		stdlib.BuiltinValues,
 		checkerOptions,
 		true,
+		importResolutionResults{},
 	)
 	if err != nil {
 		return nil, newError(err, context)
@@ -360,6 +363,7 @@ func (r *interpreterRuntime) ExecuteTransaction(script Script, context Context)
 		stdlib.BuiltinValues,
 		checkerOptions,
 		true,
+		importResolutionResults{},
 	)
 	if err != nil {
 		return newError(err, context)
@@ -587,6 +591,7 @@ func (r *interpreterRuntime) ParseAndCheckProgram(code []byte, context Context)
 		stdlib.BuiltinValues,
 		checkerOptions,
 		true,
+		importResolutionResults{},
 	)
 	if err != nil {
 		return nil, newError(err, context)
@@ -602,6 +607,7 @@ func (r *interpreterRuntime) parseAndCheckProgram(
 	values stdlib.StandardLibraryValues,
 	checkerOptions []sema.Option,
 	storeProgram bool,
+	checkedImports importResolutionResults,
 ) (
 	program *interpreter.Program,
 	err error,
@@ -639,7 +645,7 @@ func (r *interpreterRuntime) parseAndCheckProgram(
 
 	// Check
 
-	elaboration, err := r.check(parse, context, functions, values, checkerOptions)
+	elaboration, err := r.check(parse, context, functions, values, checkerOptions, checkedImports)
 	if err != nil {
 		return nil, wrapError(err)
 	}
@@ -669,6 +675,7 @@ func (r *interpreterRuntime) check(
 	functions stdlib.StandardLibraryFunctions,
 	values stdlib.StandardLibraryValues,
 	checkerOptions []sema.Option,
+	checkedImports importResolutionResults,
 ) (
 	elaboration *sema.Elaboration,
 	err error,
@@ -698,16 +705,28 @@ func (r *interpreterRuntime) check(
 					},
 				),
 				sema.WithImportHandler(
-					func(checker *sema.Checker, location common.Location) (sema.Import, error) {
+					func(checker *sema.Checker, importedLocation common.Location, importRange ast.Range) (sema.Import, error) {
+
 						var elaboration *sema.Elaboration
-						switch location {
+						switch importedLocation {
 						case stdlib.CryptoChecker.Location:
 							elaboration = stdlib.CryptoChecker.Elaboration
 
 						default:
-							context := startContext.WithLocation(location)
+							context := startContext.WithLocation(importedLocation)
+
+							// Check for cyclic imports
+							if checkedImports[importedLocation.ID()] {
+								return nil, &sema.CyclicImportsError{
+									Location: importedLocation,
+									Range:    importRange,
+								}
+							} else {
+								checkedImports[importedLocation.ID()] = true
+								defer delete(checkedImports, importedLocation.ID())
+							}
 
-							program, err := r.getProgram(context, functions, values, checkerOptions)
+							program, err := r.getProgram(context, functions, values, checkerOptions, checkedImports)
 							if err != nil {
 								return nil, err
 							}
@@ -737,9 +756,6 @@ func (r *interpreterRuntime) check(
 		return nil, err
 	}
 
-	// TODO: set elaboration *before* checking,
-	// so it is returned when there is a cyclic import
-
 	elaboration = checker.Elaboration
 
 	err = checker.Check()
@@ -863,7 +879,7 @@ func (r *interpreterRuntime) importLocationHandler(
 		default:
 			context := startContext.WithLocation(location)
 
-			program, err := r.getProgram(context, functions, values, checkerOptions)
+			program, err := r.getProgram(context, functions, values, checkerOptions, importResolutionResults{})
 			if err != nil {
 				panic(err)
 			}
@@ -887,6 +903,7 @@ func (r *interpreterRuntime) getProgram(
 	functions stdlib.StandardLibraryFunctions,
 	values stdlib.StandardLibraryValues,
 	checkerOptions []sema.Option,
+	checkedImports importResolutionResults,
 ) (
 	program *interpreter.Program,
 	err error,
@@ -914,6 +931,7 @@ func (r *interpreterRuntime) getProgram(
 			values,
 			checkerOptions,
 			true,
+			checkedImports,
 		)
 		if err != nil {
 			return nil, err
@@ -1850,6 +1868,7 @@ func (r *interpreterRuntime) newAuthAccountContractsChangeFunction(
 				stdlib.BuiltinValues,
 				checkerOptions,
 				storeProgram,
+				importResolutionResults{},
 			)
 			if err != nil {
 				// Update the code for the error pretty printing

runtime/sema/check_import_declaration.go

@@ -94,15 +94,26 @@ func (checker *Checker) importResolvedLocation(resolvedLocation ResolvedLocation
 
 	if checker.importHandler != nil {
 		var err error
-		imp, err = checker.importHandler(checker, location)
+		imp, err = checker.importHandler(checker, location, locationRange)
 		if err != nil {
-			checker.report(
-				&ImportedProgramError{
+
+			// The import handler may return CyclicImportsError specifically
+			// to indicate that this import is a cyclic import.
+			// In that case, return the error as is, for this location.
+			//
+			// If the error is not a cyclic error,
+			// it is considered a error in the imported program,
+			// and is wrapped
+
+			if _, ok := err.(*CyclicImportsError); !ok {
+				err = &ImportedProgramError{
 					Err:      err,
 					Location: location,
 					Range:    locationRange,
-				},
-			)
+				}
+			}
+
+			checker.report(err)
 			return
 		}
 	}

runtime/sema/checker.go

@@ -74,7 +74,7 @@ type ResolvedLocation struct {
 
 type LocationHandlerFunc func(identifiers []ast.Identifier, location common.Location) ([]ResolvedLocation, error)
 
-type ImportHandlerFunc func(checker *Checker, location common.Location) (Import, error)
+type ImportHandlerFunc func(checker *Checker, importedLocation common.Location, importRange ast.Range) (Import, error)
 
 // Checker
 

runtime/sema/errors.go

@@ -1131,11 +1131,7 @@ func (e *ImportedProgramError) ImportLocation() common.Location {
 }
 
 func (e *ImportedProgramError) ChildErrors() []error {
-	parentErr, ok := e.Err.(errors.ParentError)
-	if !ok {
-		return nil
-	}
-	return parentErr.ChildErrors()
+	return []error{e.Err}
 }
 
 func (*ImportedProgramError) isSemanticError() {}

runtime/tests/checker/access_test.go

@@ -1642,7 +1642,7 @@ func TestCheckAccessImportGlobalValue(t *testing.T) {
 						Options: []sema.Option{
 							sema.WithAccessCheckMode(checkMode),
 							sema.WithImportHandler(
-								func(checker *sema.Checker, location common.Location) (sema.Import, error) {
+								func(_ *sema.Checker, _ common.Location, _ ast.Range) (sema.Import, error) {
 									return sema.ElaborationImport{
 										Elaboration: importedChecker.Elaboration,
 									}, nil
@@ -1849,7 +1849,7 @@ func TestCheckAccessImportGlobalValueAssignmentAndSwap(t *testing.T) {
 					Options: []sema.Option{
 						sema.WithAccessCheckMode(checkMode),
 						sema.WithImportHandler(
-							func(checker *sema.Checker, location common.Location) (sema.Import, error) {
+							func(_ *sema.Checker, _ common.Location, _ ast.Range) (sema.Import, error) {
 								return sema.ElaborationImport{
 									Elaboration: imported.Elaboration,
 								}, nil
@@ -1895,7 +1895,7 @@ func TestCheckAccessImportGlobalValueVariableDeclarationWithSecondValue(t *testi
 		ParseAndCheckOptions{
 			Options: []sema.Option{
 				sema.WithImportHandler(
-					func(checker *sema.Checker, location common.Location) (sema.Import, error) {
+					func(_ *sema.Checker, _ common.Location, _ ast.Range) (sema.Import, error) {
 						return sema.ElaborationImport{
 							Elaboration: imported.Elaboration,
 						}, nil
@@ -2382,7 +2382,7 @@ func TestCheckAccountAccess(t *testing.T) {
 									Options: []sema.Option{
 										sema.WithAccessCheckMode(checkMode),
 										sema.WithImportHandler(
-											func(checker *sema.Checker, location common.Location) (sema.Import, error) {
+											func(_ *sema.Checker, _ common.Location, _ ast.Range) (sema.Import, error) {
 												return sema.ElaborationImport{
 													Elaboration: importedChecker.Elaboration,
 												}, nil

runtime/tests/checker/events_test.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/onflow/cadence/runtime/ast"
 	"github.com/onflow/cadence/runtime/common"
 	"github.com/onflow/cadence/runtime/errors"
 	"github.com/onflow/cadence/runtime/tests/utils"
@@ -266,7 +267,7 @@ func TestCheckEmitEvent(t *testing.T) {
 			ParseAndCheckOptions{
 				Options: []sema.Option{
 					sema.WithImportHandler(
-						func(checker *sema.Checker, location common.Location) (sema.Import, error) {
+						func(_ *sema.Checker, _ common.Location, _ ast.Range) (sema.Import, error) {
 							return sema.ElaborationImport{
 								Elaboration: importedChecker.Elaboration,
 							}, nil