Skip to content

Commit

Permalink
fix bugs found during php-corpus check (#273)
Browse files Browse the repository at this point in the history
Running NoVerify on the big open source code corpus
revealed several bugs:

1. GetRootNode now returns *node.Root instead of node.Node,
   we assigned that to node.Node vairable and nil check
   was broken, causing linter to panic on nil nodes
   instead of giving "empty root node" notice.

2. Calling EnterNode on anon class gives a panic,
   since class name is nil and we can't set
   current class name because of that.
   Since a lot of class handling relies on
   current class name being available,
   skip anon class handling for now by
   returning false for these nodes.
   This cludge can be removed when #272 is resolved.

3. We can't eagerly resolve ClassConstFetch if
   ExprType is called when indexing is incomplete
   as it uses FindClass function that depends on meta.Index
   being complete. This causes race condition in fact
   (concurrent map reads and writes). Fixed by
   introducing wrapped class const fetch type.

Signed-off-by: Iskander Sharipov <[email protected]>
  • Loading branch information
quasilyte authored Oct 31, 2019
1 parent ec5993d commit 3a7013d
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 8 deletions.
3 changes: 2 additions & 1 deletion src/linter/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import (
// 27 - added Static field to meta.FuncInfo
// 28 - array type parsed as mixed[]
// 29 - updated type inference for ClassConstFetch
const cacheVersion = 29
// 30 - resolve ClassConstFetch to a wrapped type string
const cacheVersion = 30

var (
errWrongVersion = errors.New("Wrong cache version")
Expand Down
6 changes: 3 additions & 3 deletions src/linter/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,16 +108,16 @@ func ParseContents(filename string, contents []byte, lineRanges []git.LineRange)
return analyzeFile(filename, bufCopy, parser, lineRanges)
}

func analyzeFile(filename string, contents []byte, parser *php7.Parser, lineRanges []git.LineRange) (rootNode node.Node, w *RootWalker, err error) {
func analyzeFile(filename string, contents []byte, parser *php7.Parser, lineRanges []git.LineRange) (*node.Root, *RootWalker, error) {
start := time.Now()
rootNode = parser.GetRootNode()
rootNode := parser.GetRootNode()

if rootNode == nil {
lintdebug.Send("Could not parse %s at all due to errors", filename)
return nil, nil, errors.New("Empty root node")
}

w = &RootWalker{
w := &RootWalker{
filename: filename,
lineRanges: lineRanges,
st: &meta.ClassParseState{},
Expand Down
5 changes: 5 additions & 0 deletions src/linter/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,11 @@ func (d *RootWalker) EnterNode(w walker.Walkable) (res bool) {
}
}

if class, ok := w.(*stmt.Class); ok && class.ClassName == nil {
// TODO: remove when #62 and anon class support in general is ready.
return false // Don't walk nor enter anon classes
}

state.EnterNode(d.st, w)

switch n := w.(type) {
Expand Down
16 changes: 16 additions & 0 deletions src/meta/typestring.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ const (
// Params: [Class name <string>] [Property name with $ <string>]
WStaticPropertyFetch

// WClassConstFetch is a const fetch from a class.
// E.g. Test::CONSTANT
// Params: [Class name <string>] [Constant name <string>]
WClassConstFetch

// WInstancePropertyFetch is a property fetch from some instance.
// You need to provide expression type, see example for WInstanceMethodCall.
// E.g. $var->something
Expand Down Expand Up @@ -176,6 +181,14 @@ func UnwrapInstanceMethodCall(s string) (typ, methodName string) {
return unwrap2(s)
}

func WrapClassConstFetch(className, constName string) string {
return wrap(WClassConstFetch, nil, className, constName)
}

func UnwrapClassConstFetch(s string) (className, constName string) {
return unwrap2(s)
}

func WrapStaticPropertyFetch(className, propName string) string {
if !strings.HasPrefix(propName, "$") {
propName = "$" + propName
Expand Down Expand Up @@ -277,6 +290,9 @@ func formatType(s string) (res string) {
case WStaticPropertyFetch:
className, propertyName := UnwrapStaticPropertyFetch(s)
return className + "::" + propertyName
case WClassConstFetch:
className, constName := UnwrapClassConstFetch(s)
return className + "::" + constName
}

return "unknown(" + s + ")"
Expand Down
5 changes: 1 addition & 4 deletions src/solver/exprtype.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,7 @@ func exprTypeLocalCustom(sc *meta.Scope, cs *meta.ClassParseState, n node.Node,
if !ok {
return meta.TypesMap{}
}
res, _, ok := FindConstant(className, n.ConstantName.Value)
if ok {
return res.Typ
}
return meta.NewTypesMap(meta.WrapClassConstFetch(className, n.ConstantName.Value))
case *expr.ConstFetch:
nm, ok := n.Constant.(*name.Name)
if !ok {
Expand Down
6 changes: 6 additions & 0 deletions src/solver/solver.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,12 @@ func (r *resolver) resolveTypeNoLateStaticBinding(class, typ string) map[string]
if ok {
return r.resolveTypes(class, info.Typ)
}
case meta.WClassConstFetch:
className, constName := meta.UnwrapClassConstFetch(typ)
info, _, ok := FindConstant(className, constName)
if ok {
return r.resolveTypes(class, info.Typ)
}
default:
panic(fmt.Sprintf("Unexpected type: %d", typ[0]))
}
Expand Down

0 comments on commit 3a7013d

Please sign in to comment.