Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggested fixes for pub and priv parser errors #2767

Merged
merged 3 commits into from
Sep 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions runtime/ast/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
11 changes: 11 additions & 0 deletions runtime/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
turbolent marked this conversation as resolved.
Show resolved Hide resolved
}

var _ UserError = MemoryError{}

func (MemoryError) IsUserError() {}
Expand Down
14 changes: 10 additions & 4 deletions runtime/parser/declaration.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,11 @@
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 {
Expand Down Expand Up @@ -1661,8 +1664,11 @@
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)`")

Check warning on line 1668 in runtime/parser/declaration.go

View check run for this annotation

Codecov / codecov/patch

runtime/parser/declaration.go#L1667-L1668

Added lines #L1667 - L1668 were not covered by tests

case KeywordPriv:
return nil, p.syntaxErrorWithSuggestedFix("`priv` is no longer a valid access keyword", "`access(self)`")

Check warning on line 1671 in runtime/parser/declaration.go

View check run for this annotation

Codecov / codecov/patch

runtime/parser/declaration.go#L1670-L1671

Added lines #L1670 - L1671 were not covered by tests

case KeywordAccess:
if access != ast.AccessNotSpecified {
Expand Down
24 changes: 18 additions & 6 deletions runtime/parser/declaration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
42 changes: 42 additions & 0 deletions runtime/parser/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,48 @@
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

Check warning on line 119 in runtime/parser/errors.go

View check run for this annotation

Codecov / codecov/patch

runtime/parser/errors.go#L118-L119

Added lines #L118 - L119 were not covered by tests
}

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

Check warning on line 133 in runtime/parser/errors.go

View check run for this annotation

Codecov / codecov/patch

runtime/parser/errors.go#L122-L133

Added lines #L122 - L133 were not covered by tests
}

// JuxtaposedUnaryOperatorsError

type JuxtaposedUnaryOperatorsError struct {
Expand Down
4 changes: 4 additions & 0 deletions runtime/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
45 changes: 14 additions & 31 deletions runtime/sema/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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() {}

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

Expand All @@ -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,
Expand All @@ -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{
Expand Down Expand Up @@ -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() {}

Expand Down Expand Up @@ -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{
Expand Down
3 changes: 1 addition & 2 deletions runtime/tests/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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("")
}
}
Expand Down
6 changes: 3 additions & 3 deletions tools/analysis/diagnostic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading