From 8a5f27d504c8791c20ac64b20e6d461c2d1fd8af Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Thu, 7 Sep 2023 14:25:28 -0400 Subject: [PATCH 1/3] refactor HasSuggestedFixes out of sema --- runtime/ast/ast.go | 6 +++++ runtime/errors/errors.go | 11 +++++++++ runtime/parser/errors.go | 35 ++++++++++++++++++++++++++++ runtime/sema/errors.go | 45 +++++++++++------------------------- runtime/tests/utils/utils.go | 3 +-- tools/analysis/diagnostic.go | 6 ++--- 6 files changed, 70 insertions(+), 36 deletions(-) diff --git a/runtime/ast/ast.go b/runtime/ast/ast.go index b44be438fe..de9ecfce4a 100644 --- a/runtime/ast/ast.go +++ b/runtime/ast/ast.go @@ -23,3 +23,9 @@ // Elements also implement the json.Marshaler interface // so can be serialized to a standardized/stable JSON format. package ast + +type TextEdit struct { + Replacement string + Insertion string + Range +} diff --git a/runtime/errors/errors.go b/runtime/errors/errors.go index e5ec4e7c30..0a298a88dc 100644 --- a/runtime/errors/errors.go +++ b/runtime/errors/errors.go @@ -114,6 +114,17 @@ type MemoryError struct { Err error } +// SuggestedFix + +type HasSuggestedFixes[T any] interface { + SuggestFixes(code string) []SuggestedFix[T] +} + +type SuggestedFix[T any] struct { + Message string + TextEdits []T +} + var _ UserError = MemoryError{} func (MemoryError) IsUserError() {} diff --git a/runtime/parser/errors.go b/runtime/parser/errors.go index 90dff88f42..5433cb6fb0 100644 --- a/runtime/parser/errors.go +++ b/runtime/parser/errors.go @@ -91,6 +91,41 @@ func (e *SyntaxError) Error() string { return e.Message } +// SyntaxErrorWithSuggestedFix + +type SyntaxErrorWithSuggestedFix struct { + Message string + SuggestedFix string + Pos ast.Position +} + +func NewSyntaxErrorWithSuggestedFix(pos ast.Position, message string, suggestedFix string) *SyntaxErrorWithSuggestedFix { + return &SyntaxErrorWithSuggestedFix{ + Pos: pos, + Message: message, + SuggestedFix: suggestedFix, + } +} + +var _ ParseError = &SyntaxErrorWithSuggestedFix{} +var _ errors.UserError = &SyntaxErrorWithSuggestedFix{} + +func (*SyntaxErrorWithSuggestedFix) isParseError() {} + +func (*SyntaxErrorWithSuggestedFix) IsUserError() {} + +func (e *SyntaxErrorWithSuggestedFix) StartPosition() ast.Position { + return e.Pos +} + +func (e *SyntaxErrorWithSuggestedFix) EndPosition(_ common.MemoryGauge) ast.Position { + return e.Pos +} + +func (e *SyntaxErrorWithSuggestedFix) Error() string { + return e.Message +} + // JuxtaposedUnaryOperatorsError type JuxtaposedUnaryOperatorsError struct { diff --git a/runtime/sema/errors.go b/runtime/sema/errors.go index 1c85426f38..d15dce1be6 100644 --- a/runtime/sema/errors.go +++ b/runtime/sema/errors.go @@ -77,23 +77,6 @@ func (e *unsupportedOperation) Error() string { ) } -// SuggestedFix - -type HasSuggestedFixes interface { - SuggestFixes(code string) []SuggestedFix -} - -type SuggestedFix struct { - Message string - TextEdits []TextEdit -} - -type TextEdit struct { - Replacement string - Insertion string - ast.Range -} - // InvalidPragmaError type InvalidPragmaError struct { @@ -496,7 +479,7 @@ type MissingArgumentLabelError struct { var _ SemanticError = &MissingArgumentLabelError{} var _ errors.UserError = &MissingArgumentLabelError{} -var _ HasSuggestedFixes = &MissingArgumentLabelError{} +var _ errors.HasSuggestedFixes[ast.TextEdit] = &MissingArgumentLabelError{} func (*MissingArgumentLabelError) isSemanticError() {} @@ -509,11 +492,11 @@ func (e *MissingArgumentLabelError) Error() string { ) } -func (e *MissingArgumentLabelError) SuggestFixes(_ string) []SuggestedFix { - return []SuggestedFix{ +func (e *MissingArgumentLabelError) SuggestFixes(_ string) []errors.SuggestedFix[ast.TextEdit] { + return []errors.SuggestedFix[ast.TextEdit]{ { Message: "insert argument label", - TextEdits: []TextEdit{ + TextEdits: []ast.TextEdit{ { Insertion: fmt.Sprintf("%s: ", e.ExpectedArgumentLabel), Range: ast.NewUnmeteredRange( @@ -537,7 +520,7 @@ type IncorrectArgumentLabelError struct { var _ SemanticError = &IncorrectArgumentLabelError{} var _ errors.UserError = &IncorrectArgumentLabelError{} var _ errors.SecondaryError = &IncorrectArgumentLabelError{} -var _ HasSuggestedFixes = &IncorrectArgumentLabelError{} +var _ errors.HasSuggestedFixes[ast.TextEdit] = &IncorrectArgumentLabelError{} func (*IncorrectArgumentLabelError) isSemanticError() {} @@ -559,12 +542,12 @@ func (e *IncorrectArgumentLabelError) SecondaryError() string { ) } -func (e *IncorrectArgumentLabelError) SuggestFixes(code string) []SuggestedFix { +func (e *IncorrectArgumentLabelError) SuggestFixes(code string) []errors.SuggestedFix[ast.TextEdit] { if len(e.ExpectedArgumentLabel) > 0 { - return []SuggestedFix{ + return []errors.SuggestedFix[ast.TextEdit]{ { Message: "replace argument label", - TextEdits: []TextEdit{ + TextEdits: []ast.TextEdit{ { Replacement: fmt.Sprintf("%s:", e.ExpectedArgumentLabel), Range: e.Range, @@ -586,10 +569,10 @@ func (e *IncorrectArgumentLabelError) SuggestFixes(code string) []SuggestedFix { adjustedEndPos := endPos.Shifted(nil, whitespaceSuffixLength) - return []SuggestedFix{ + return []errors.SuggestedFix[ast.TextEdit]{ { Message: "remove argument label", - TextEdits: []TextEdit{ + TextEdits: []ast.TextEdit{ { Replacement: "", Range: ast.Range{ @@ -1024,7 +1007,7 @@ type NotDeclaredMemberError struct { var _ SemanticError = &NotDeclaredMemberError{} var _ errors.UserError = &NotDeclaredMemberError{} var _ errors.SecondaryError = &NotDeclaredMemberError{} -var _ HasSuggestedFixes = &NotDeclaredMemberError{} +var _ errors.HasSuggestedFixes[ast.TextEdit] = &NotDeclaredMemberError{} func (*NotDeclaredMemberError) isSemanticError() {} @@ -1064,7 +1047,7 @@ func (e *NotDeclaredMemberError) SecondaryError() string { return "unknown member" } -func (e *NotDeclaredMemberError) SuggestFixes(_ string) []SuggestedFix { +func (e *NotDeclaredMemberError) SuggestFixes(_ string) []errors.SuggestedFix[ast.TextEdit] { optionalMember := e.findOptionalMember() if optionalMember == "" { return nil @@ -1072,10 +1055,10 @@ func (e *NotDeclaredMemberError) SuggestFixes(_ string) []SuggestedFix { accessPos := e.Expression.AccessPos - return []SuggestedFix{ + return []errors.SuggestedFix[ast.TextEdit]{ { Message: "use optional chaining", - TextEdits: []TextEdit{ + TextEdits: []ast.TextEdit{ { Insertion: "?", Range: ast.Range{ diff --git a/runtime/tests/utils/utils.go b/runtime/tests/utils/utils.go index 8fcdf5d306..ce62a55b81 100644 --- a/runtime/tests/utils/utils.go +++ b/runtime/tests/utils/utils.go @@ -32,7 +32,6 @@ import ( "github.com/onflow/cadence/runtime/ast" "github.com/onflow/cadence/runtime/errors" "github.com/onflow/cadence/runtime/interpreter" - "github.com/onflow/cadence/runtime/sema" "github.com/onflow/cadence/runtime/common" ) @@ -234,7 +233,7 @@ func RequireError(t *testing.T, err error) { _ = hasSecondaryError.SecondaryError() } - if hasSuggestedFixes, ok := err.(sema.HasSuggestedFixes); ok { + if hasSuggestedFixes, ok := err.(errors.HasSuggestedFixes[ast.TextEdit]); ok { _ = hasSuggestedFixes.SuggestFixes("") } } diff --git a/tools/analysis/diagnostic.go b/tools/analysis/diagnostic.go index 5a6179dbcd..2eb21cc85d 100644 --- a/tools/analysis/diagnostic.go +++ b/tools/analysis/diagnostic.go @@ -21,12 +21,12 @@ package analysis import ( "github.com/onflow/cadence/runtime/ast" "github.com/onflow/cadence/runtime/common" - "github.com/onflow/cadence/runtime/sema" + "github.com/onflow/cadence/runtime/errors" ) -type SuggestedFix = sema.SuggestedFix +type SuggestedFix = errors.SuggestedFix[ast.TextEdit] -type TextEdit = sema.TextEdit +type TextEdit = ast.TextEdit type Diagnostic struct { Location common.Location From 0c0345f15d0105525a126ac7ecb45a27e7eb342c Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Thu, 7 Sep 2023 14:39:31 -0400 Subject: [PATCH 2/3] suggestions for pub and priv fixes --- runtime/parser/declaration.go | 14 ++++++++++---- runtime/parser/declaration_test.go | 21 ++++++++++++--------- runtime/parser/errors.go | 19 +++++++++++++++++++ runtime/parser/parser.go | 4 ++++ 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/runtime/parser/declaration.go b/runtime/parser/declaration.go index 789aefc7c5..2bec673b50 100644 --- a/runtime/parser/declaration.go +++ b/runtime/parser/declaration.go @@ -214,8 +214,11 @@ func parseDeclaration(p *parser, docString string) (ast.Declaration, error) { purity = parsePurityAnnotation(p) continue - case KeywordPub, KeywordPriv: - return nil, p.syntaxError(fmt.Sprintf("`%s` is no longer a valid access keyword", p.currentTokenSource())) + case KeywordPub: + return nil, p.syntaxErrorWithSuggestedFix("`pub` is no longer a valid access keyword", "`access(all)`") + + case KeywordPriv: + return nil, p.syntaxErrorWithSuggestedFix("`priv` is no longer a valid access keyword", "`access(self)`") case KeywordAccess: if access != ast.AccessNotSpecified { @@ -1661,8 +1664,11 @@ func parseMemberOrNestedDeclaration(p *parser, docString string) (ast.Declaratio purity = parsePurityAnnotation(p) continue - case KeywordPub, KeywordPriv: - return nil, p.syntaxError(fmt.Sprintf("`%s` is no longer a valid access keyword", p.currentTokenSource())) + case KeywordPub: + return nil, p.syntaxErrorWithSuggestedFix("`pub` is no longer a valid access keyword", "`access(all)`") + + case KeywordPriv: + return nil, p.syntaxErrorWithSuggestedFix("`priv` is no longer a valid access keyword", "`access(self)`") case KeywordAccess: if access != ast.AccessNotSpecified { diff --git a/runtime/parser/declaration_test.go b/runtime/parser/declaration_test.go index 81aaed3223..f3c334fab6 100644 --- a/runtime/parser/declaration_test.go +++ b/runtime/parser/declaration_test.go @@ -9788,9 +9788,10 @@ func TestParseDeprecatedAccessModifiers(t *testing.T) { _, errs := testParseDeclarations(" pub fun foo ( ) { }") utils.AssertEqualWithDiff(t, []error{ - &SyntaxError{ - Message: "`pub` is no longer a valid access keyword", - Pos: ast.Position{Offset: 1, Line: 1, Column: 1}, + &SyntaxErrorWithSuggestedFix{ + Message: "`pub` is no longer a valid access keyword", + Pos: ast.Position{Offset: 1, Line: 1, Column: 1}, + SuggestedFix: "`access(all)`", }, }, errs, @@ -9805,9 +9806,10 @@ func TestParseDeprecatedAccessModifiers(t *testing.T) { _, errs := testParseDeclarations(" priv fun foo ( ) { }") utils.AssertEqualWithDiff(t, []error{ - &SyntaxError{ - Message: "`priv` is no longer a valid access keyword", - Pos: ast.Position{Offset: 1, Line: 1, Column: 1}, + &SyntaxErrorWithSuggestedFix{ + Message: "`priv` is no longer a valid access keyword", + Pos: ast.Position{Offset: 1, Line: 1, Column: 1}, + SuggestedFix: "`access(self)`", }, }, errs, @@ -9822,9 +9824,10 @@ func TestParseDeprecatedAccessModifiers(t *testing.T) { _, errs := testParseDeclarations(" pub(set) fun foo ( ) { }") utils.AssertEqualWithDiff(t, []error{ - &SyntaxError{ - Message: "`pub` is no longer a valid access keyword", - Pos: ast.Position{Offset: 1, Line: 1, Column: 1}, + &SyntaxErrorWithSuggestedFix{ + Message: "`pub` is no longer a valid access keyword", + Pos: ast.Position{Offset: 1, Line: 1, Column: 1}, + SuggestedFix: "`access(all)`", }, }, errs, diff --git a/runtime/parser/errors.go b/runtime/parser/errors.go index 5433cb6fb0..c6a1450916 100644 --- a/runtime/parser/errors.go +++ b/runtime/parser/errors.go @@ -99,6 +99,8 @@ type SyntaxErrorWithSuggestedFix struct { Pos ast.Position } +var _ errors.HasSuggestedFixes[ast.TextEdit] = &SyntaxErrorWithSuggestedFix{} + func NewSyntaxErrorWithSuggestedFix(pos ast.Position, message string, suggestedFix string) *SyntaxErrorWithSuggestedFix { return &SyntaxErrorWithSuggestedFix{ Pos: pos, @@ -126,6 +128,23 @@ func (e *SyntaxErrorWithSuggestedFix) Error() string { return e.Message } +func (e *SyntaxErrorWithSuggestedFix) SuggestFixes(_ string) []errors.SuggestedFix[ast.TextEdit] { + return []errors.SuggestedFix[ast.TextEdit]{ + { + Message: fmt.Sprintf("replace with %s", e.SuggestedFix), + TextEdits: []ast.TextEdit{ + { + Replacement: e.SuggestedFix, + Range: ast.Range{ + StartPos: e.Pos, + EndPos: e.Pos, + }, + }, + }, + }, + } +} + // JuxtaposedUnaryOperatorsError type JuxtaposedUnaryOperatorsError struct { diff --git a/runtime/parser/parser.go b/runtime/parser/parser.go index 5bfb6c3f5a..629d65cb36 100644 --- a/runtime/parser/parser.go +++ b/runtime/parser/parser.go @@ -195,6 +195,10 @@ func (p *parser) syntaxError(message string, params ...any) error { return NewSyntaxError(p.current.StartPos, message, params...) } +func (p *parser) syntaxErrorWithSuggestedFix(message string, suggestedFix string) error { + return NewSyntaxErrorWithSuggestedFix(p.current.StartPos, message, suggestedFix) +} + func (p *parser) reportSyntaxError(message string, params ...any) { err := p.syntaxError(message, params...) p.report(err) From 4ed17057fd6d7b1cc6254a67b2df67ee650123cd Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Thu, 7 Sep 2023 15:21:47 -0400 Subject: [PATCH 3/3] review comments --- runtime/parser/declaration_test.go | 27 ++++++++++++++------- runtime/parser/errors.go | 38 ++++++++++-------------------- runtime/parser/parser.go | 2 +- 3 files changed, 32 insertions(+), 35 deletions(-) diff --git a/runtime/parser/declaration_test.go b/runtime/parser/declaration_test.go index f3c334fab6..7a36b96683 100644 --- a/runtime/parser/declaration_test.go +++ b/runtime/parser/declaration_test.go @@ -9788,9 +9788,12 @@ func TestParseDeprecatedAccessModifiers(t *testing.T) { _, errs := testParseDeclarations(" pub fun foo ( ) { }") utils.AssertEqualWithDiff(t, []error{ - &SyntaxErrorWithSuggestedFix{ - Message: "`pub` is no longer a valid access keyword", - Pos: ast.Position{Offset: 1, Line: 1, Column: 1}, + &SyntaxErrorWithSuggestedReplacement{ + Message: "`pub` is no longer a valid access keyword", + Range: ast.Range{ + StartPos: ast.Position{Offset: 1, Line: 1, Column: 1}, + EndPos: ast.Position{Offset: 3, Line: 1, Column: 3}, + }, SuggestedFix: "`access(all)`", }, }, @@ -9806,9 +9809,12 @@ func TestParseDeprecatedAccessModifiers(t *testing.T) { _, errs := testParseDeclarations(" priv fun foo ( ) { }") utils.AssertEqualWithDiff(t, []error{ - &SyntaxErrorWithSuggestedFix{ - Message: "`priv` is no longer a valid access keyword", - Pos: ast.Position{Offset: 1, Line: 1, Column: 1}, + &SyntaxErrorWithSuggestedReplacement{ + Message: "`priv` is no longer a valid access keyword", + Range: ast.Range{ + StartPos: ast.Position{Offset: 1, Line: 1, Column: 1}, + EndPos: ast.Position{Offset: 4, Line: 1, Column: 4}, + }, SuggestedFix: "`access(self)`", }, }, @@ -9824,9 +9830,12 @@ func TestParseDeprecatedAccessModifiers(t *testing.T) { _, errs := testParseDeclarations(" pub(set) fun foo ( ) { }") utils.AssertEqualWithDiff(t, []error{ - &SyntaxErrorWithSuggestedFix{ - Message: "`pub` is no longer a valid access keyword", - Pos: ast.Position{Offset: 1, Line: 1, Column: 1}, + &SyntaxErrorWithSuggestedReplacement{ + Message: "`pub` is no longer a valid access keyword", + Range: ast.Range{ + StartPos: ast.Position{Offset: 1, Line: 1, Column: 1}, + EndPos: ast.Position{Offset: 3, Line: 1, Column: 3}, + }, SuggestedFix: "`access(all)`", }, }, diff --git a/runtime/parser/errors.go b/runtime/parser/errors.go index c6a1450916..a6e5944005 100644 --- a/runtime/parser/errors.go +++ b/runtime/parser/errors.go @@ -93,52 +93,40 @@ func (e *SyntaxError) Error() string { // SyntaxErrorWithSuggestedFix -type SyntaxErrorWithSuggestedFix struct { +type SyntaxErrorWithSuggestedReplacement struct { Message string SuggestedFix string - Pos ast.Position + ast.Range } -var _ errors.HasSuggestedFixes[ast.TextEdit] = &SyntaxErrorWithSuggestedFix{} +var _ errors.HasSuggestedFixes[ast.TextEdit] = &SyntaxErrorWithSuggestedReplacement{} -func NewSyntaxErrorWithSuggestedFix(pos ast.Position, message string, suggestedFix string) *SyntaxErrorWithSuggestedFix { - return &SyntaxErrorWithSuggestedFix{ - Pos: pos, +func NewSyntaxErrorWithSuggestedReplacement(r ast.Range, message string, suggestedFix string) *SyntaxErrorWithSuggestedReplacement { + return &SyntaxErrorWithSuggestedReplacement{ + Range: r, Message: message, SuggestedFix: suggestedFix, } } -var _ ParseError = &SyntaxErrorWithSuggestedFix{} -var _ errors.UserError = &SyntaxErrorWithSuggestedFix{} - -func (*SyntaxErrorWithSuggestedFix) isParseError() {} - -func (*SyntaxErrorWithSuggestedFix) IsUserError() {} - -func (e *SyntaxErrorWithSuggestedFix) StartPosition() ast.Position { - return e.Pos -} +var _ ParseError = &SyntaxErrorWithSuggestedReplacement{} +var _ errors.UserError = &SyntaxErrorWithSuggestedReplacement{} -func (e *SyntaxErrorWithSuggestedFix) EndPosition(_ common.MemoryGauge) ast.Position { - return e.Pos -} +func (*SyntaxErrorWithSuggestedReplacement) isParseError() {} -func (e *SyntaxErrorWithSuggestedFix) Error() string { +func (*SyntaxErrorWithSuggestedReplacement) IsUserError() {} +func (e *SyntaxErrorWithSuggestedReplacement) Error() string { return e.Message } -func (e *SyntaxErrorWithSuggestedFix) SuggestFixes(_ string) []errors.SuggestedFix[ast.TextEdit] { +func (e *SyntaxErrorWithSuggestedReplacement) SuggestFixes(_ string) []errors.SuggestedFix[ast.TextEdit] { return []errors.SuggestedFix[ast.TextEdit]{ { Message: fmt.Sprintf("replace with %s", e.SuggestedFix), TextEdits: []ast.TextEdit{ { Replacement: e.SuggestedFix, - Range: ast.Range{ - StartPos: e.Pos, - EndPos: e.Pos, - }, + Range: e.Range, }, }, }, diff --git a/runtime/parser/parser.go b/runtime/parser/parser.go index 629d65cb36..37c86a9b89 100644 --- a/runtime/parser/parser.go +++ b/runtime/parser/parser.go @@ -196,7 +196,7 @@ func (p *parser) syntaxError(message string, params ...any) error { } func (p *parser) syntaxErrorWithSuggestedFix(message string, suggestedFix string) error { - return NewSyntaxErrorWithSuggestedFix(p.current.StartPos, message, suggestedFix) + return NewSyntaxErrorWithSuggestedReplacement(p.current.Range, message, suggestedFix) } func (p *parser) reportSyntaxError(message string, params ...any) {