metadatapath analyzer is being skipped

[eh-am]

Hi, the reviewer caught this error when I submitted to grafana, but I expected it to be caught way earlier in my ci lint step:

[must fix] The logo in the datasource list is broken. The path in plugin.json point to a non-existing file. Please include a logo in the zip file and/or adjust plugin.json accordingly:

Which seems to be analyzed by metadatapath:

plugin-validator/pkg/analysis/passes/metadatapaths/metadatapaths.go

Line 113 in facec49

pass.ReportResult(pass.AnalyzerName, pathNotExists, fmt.Sprintf("plugin.json: %s path doesn't exists: %s", path.kind, path.path), "Refer only existing files. Make sure the files referred in plugin.json are included in the archive.")

Debugging a little bit, I found that it was simply not being run!

For illustration, use the following config.yaml file:

global:
  enabled: false
  jsonOutput: false
  reportAll: true

analyzers:
  metadatapaths:
    enabled: true

I also found that removing the following loop "fixes" the issue:

plugin-validator/pkg/runner/runner.go

Lines 82 to 86 in facec49

	for _, dep := range a.Requires {
	if pass.ResultOf[dep] == nil {
	return nil
	}
	}

I didn't look too much so I don't know how that affects other places.

[eh-am]

Just to give some more information, it happens because I don't have any screenshots, so it returns nil here

plugin-validator/pkg/analysis/passes/screenshots/screenshots.go

Line 34 in 6da5c1c

return nil, nil

Which then forces metadatapath to be skipped here

plugin-validator/pkg/runner/runner.go

Line 83 in 6da5c1c

if pass.ResultOf[dep] == nil {

[academo]

Hi @eh-am thanks for the bug report. Do you have a plugin archive file I can use to test this?

It seems the loop you mention seems to be indeed the problem but just removing it might bring problems for other validators so I want to check first the case and see if we can fix it somehow else.

[eh-am]

Hi @academo, here's an example.

grafana-datasource-plugin-1.2.0.zip

I disabled all analyzers except metadatapaths.Analyzer in analysis/passes/analysis.go to make it easier to debug.

As I mentioned in the previous message (#101 (comment)) the problem seems to come from the screenshot analyzer, which returns null, making the metadatapath analyzer to return early.

This is plugincheck2 btw.

[academo]

Thanks @eh-am I'll take a look. You should also feel free to open a PR if you want to fix this issue. Be mindful that removing the loop might not be the best option. My idea so far is to return an empty array in the screenshots analyzer.

Generally speaking I don't like our current use of interface{} in the analzyers return types and we might need to restructure all of it to use generics or retrospection. That's a refactor I haven't found the time to work on yet.