Run validators with dependencies returning nil
#147
Conversation
| @@ -22,8 +22,15 @@ var Analyzer = &analysis.Analyzer{ | |||
| } | |||
|
|
|||
| func run(pass *analysis.Pass) (interface{}, error) { | |||
| metadataBody := pass.ResultOf[metadata.Analyzer].([]byte) | |||
| archiveDir := pass.ResultOf[archive.Analyzer].(string) | |||
| metadataBody, ok := pass.ResultOf[metadata.Analyzer].([]byte) | |||
There was a problem hiding this comment.
most of the bulk of the changes in this PR are adding these validations for type castings.
| @@ -35,7 +35,7 @@ var Analyzer = &analysis.Analyzer{ | |||
| func run(pass *analysis.Pass) (interface{}, error) { | |||
| metadataBody, ok := pass.ResultOf[metadata.Analyzer].([]byte) | |||
| if !ok { | |||
| return nil, errors.New("metadata not found") | |||
| return nil, nil | |||
There was a problem hiding this comment.
This change is intended. We don't want to fail the process with this. Instead we should simply skip the validator. the metadata validator is already responsible of reporting this error so this is redundant logic.
| @@ -54,6 +61,12 @@ func run(pass *analysis.Pass) (interface{}, error) { | |||
| return nil, err | |||
| } | |||
|
|
|||
| _, err = os.Stat(metadataPath) | |||
There was a problem hiding this comment.
before this would not even execute, now it executes and report a missing plugin.json
| @@ -24,7 +23,7 @@ func run(pass *analysis.Pass) (interface{}, error) { | |||
| archiveDir, ok := pass.ResultOf[archive.Analyzer].(string) | |||
| if !ok || archiveDir == "" { | |||
| // this should never happen | |||
| return nil, fmt.Errorf("archive dir not found") | |||
| return nil, nil | |||
There was a problem hiding this comment.
Intended. The error should come from the archive validator, this is redundant logic,. we can skip this validator if there's no archive
| } | ||
| } | ||
|
|
||
| // TODO: Is there a better way to skip downstream analyzers than based |
There was a problem hiding this comment.
This is the key change in this PR. removing this logic that skips validators with dependencies returning nil, Now all validators are run
Co-authored-by: Giuseppe Guerra <giuseppe.guerra@grafana.com>
The original runner logic will skip validators that have dependencies returning nil.
This was under the idea that validator's dependencies are always required ("hard" dependencies)
Some new validators have "Soft" dependencies which means they can still run some checks even if the dependencies are nil
Additionally the code for many validators was not safe because it was optimistic about the result of the validator's dependencies and could cause panics.
This PR also fixes the code in all the validators that were optimistic about the result of the validator's dependencies.