Merge pull request #2767 from onflow/sainati/suggested-fixes-in-parser

Suggested fixes for `pub` and `priv` parser errors

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
+}

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() {}

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 {

runtime/parser/declaration_test.go

@@ -9788,9 +9788,13 @@ func TestParseDeprecatedAccessModifiers(t *testing.T) {
 		_, errs := testParseDeclarations(" pub fun foo ( ) { }")
 		utils.AssertEqualWithDiff(t,
 			[]error{
-				&SyntaxError{
+				&SyntaxErrorWithSuggestedReplacement{
 					Message: "`pub` is no longer a valid access keyword",
-					Pos:     ast.Position{Offset: 1, Line: 1, Column: 1},
+					Range: ast.Range{
+						StartPos: ast.Position{Offset: 1, Line: 1, Column: 1},
+						EndPos:   ast.Position{Offset: 3, Line: 1, Column: 3},
+					},
+					SuggestedFix: "`access(all)`",
 				},
 			},
 			errs,
@@ -9805,9 +9809,13 @@ func TestParseDeprecatedAccessModifiers(t *testing.T) {
 		_, errs := testParseDeclarations(" priv fun foo ( ) { }")
 		utils.AssertEqualWithDiff(t,
 			[]error{
-				&SyntaxError{
+				&SyntaxErrorWithSuggestedReplacement{
 					Message: "`priv` is no longer a valid access keyword",
-					Pos:     ast.Position{Offset: 1, Line: 1, Column: 1},
+					Range: ast.Range{
+						StartPos: ast.Position{Offset: 1, Line: 1, Column: 1},
+						EndPos:   ast.Position{Offset: 4, Line: 1, Column: 4},
+					},
+					SuggestedFix: "`access(self)`",
 				},
 			},
 			errs,
@@ -9822,9 +9830,13 @@ func TestParseDeprecatedAccessModifiers(t *testing.T) {
 		_, errs := testParseDeclarations(" pub(set) fun foo ( ) { }")
 		utils.AssertEqualWithDiff(t,
 			[]error{
-				&SyntaxError{
+				&SyntaxErrorWithSuggestedReplacement{
 					Message: "`pub` is no longer a valid access keyword",
-					Pos:     ast.Position{Offset: 1, Line: 1, Column: 1},
+					Range: ast.Range{
+						StartPos: ast.Position{Offset: 1, Line: 1, Column: 1},
+						EndPos:   ast.Position{Offset: 3, Line: 1, Column: 3},
+					},
+					SuggestedFix: "`access(all)`",
 				},
 			},
 			errs,

runtime/parser/errors.go

@@ -91,6 +91,48 @@ func (e *SyntaxError) Error() string {
 	return e.Message
 }
 
+// SyntaxErrorWithSuggestedFix
+
+type SyntaxErrorWithSuggestedReplacement struct {
+	Message      string
+	SuggestedFix string
+	ast.Range
+}
+
+var _ errors.HasSuggestedFixes[ast.TextEdit] = &SyntaxErrorWithSuggestedReplacement{}
+
+func NewSyntaxErrorWithSuggestedReplacement(r ast.Range, message string, suggestedFix string) *SyntaxErrorWithSuggestedReplacement {
+	return &SyntaxErrorWithSuggestedReplacement{
+		Range:        r,
+		Message:      message,
+		SuggestedFix: suggestedFix,
+	}
+}
+
+var _ ParseError = &SyntaxErrorWithSuggestedReplacement{}
+var _ errors.UserError = &SyntaxErrorWithSuggestedReplacement{}
+
+func (*SyntaxErrorWithSuggestedReplacement) isParseError() {}
+
+func (*SyntaxErrorWithSuggestedReplacement) IsUserError() {}
+func (e *SyntaxErrorWithSuggestedReplacement) Error() string {
+	return e.Message
+}
+
+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:       e.Range,
+				},
+			},
+		},
+	}
+}
+
 // JuxtaposedUnaryOperatorsError
 
 type JuxtaposedUnaryOperatorsError struct {

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 NewSyntaxErrorWithSuggestedReplacement(p.current.Range, message, suggestedFix)
+}
+
 func (p *parser) reportSyntaxError(message string, params ...any) {
 	err := p.syntaxError(message, params...)
 	p.report(err)

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,18 +1047,18 @@ 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
 	}
 
 	accessPos := e.Expression.AccessPos
 
-	return []SuggestedFix{
+	return []errors.SuggestedFix[ast.TextEdit]{
 		{
 			Message: "use optional chaining",
-			TextEdits: []TextEdit{
+			TextEdits: []ast.TextEdit{
 				{
 					Insertion: "?",
 					Range: ast.Range{

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("")
 	}
 }

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