api-linter icon indicating copy to clipboard operation
api-linter copied to clipboard

why not design unmarshal for lint.Response

Open BarryfwuTencent opened this issue 3 years ago • 2 comments

Sometimes, we need parse the output of api-linter in our CI pipelines, however, lint.Response don't support unmarshal, we suggest to add unmarshal in lint.Response, the code is as following,

package main

import ( "encoding/json" "fmt" )

// position describes a one-based position in a source code file. // They are one-indexed, as a human counts lines or columns. type Position struct { Line int json:"line_number" yaml:"line_number" Column int json:"column_number" yaml:"column_number" }

// fileLocation describes a location in a source code file. // // Note: Positions are one-indexed, as a human counts lines or columns // in a file. type FileLocation struct { Start Position json:"start_position" yaml:"start_position" End Position json:"end_position" yaml:"end_position" }

type Problem struct { Message string json:"message" yaml:"message" Suggestion string json:"suggestion,omitempty" yaml:"suggestion,omitempty" Location FileLocation json:"location" yaml:"location" RuleID string json:"rule_id" yaml:"rule_id" RuleDocURI string json:"rule_doc_uri" yaml:"rule_doc_uri" Category string json:"category,omitempty" yaml:"category,omitempty" }

type Response struct { FilePath string json:"file_path" yaml:"file_path" Problems []Problem json:"problems" yaml:"problems" }

func main() { var res []Response by := []byte([{ "file_path": "proto/commitmeta.proto", "problems": [ { "message": "Proto files should set option java_outer_classname = \"CommitmetaProto\".", "location": { "start_position": { "line_number": 63, "column_number": 1 }, "end_position": { "line_number": 63, "column_number": 38 } }, "rule_id": "core::0191::java-outer-classname", "rule_doc_uri": "https://linter.aip.dev/191/java-outer-classname" } ] }]) err := json.Unmarshal(by, &res) if err == nil { fmt.Printf("unmarshal: %v", res) } }

BarryfwuTencent avatar Nov 01 '22 05:11 BarryfwuTencent

To be honest I don't know why the JSON/YAML tags were removed from Problem, but they were a while back in #195. My guess is a desire to differentiate the internal Problem schema for rule developers from what the linter emitted as findings.

Can you discuss your use case some more, please?

FWIW I am looking at our internal usage of the linter and it seems like we have defined our own copies of the structs you mentioned with the JSON/YAML tags so that our CI can parse the output of the linter. I think this might be worth exporting to here if it will help you/others.

Worth mentioning that you can define these types yourself as well, they don't change really ever, and won't change in an incompatible way.

noahdietz avatar Nov 01 '22 19:11 noahdietz

To be honest I don't know why the JSON/YAML tags were removed from Problem, but they were a while back in #195. My guess is a desire to differentiate the internal Problem schema for rule developers from what the linter emitted as findings.

Can you discuss your use case some more, please?

FWIW I am looking at our internal usage of the linter and it seems like we have defined our own copies of the structs you mentioned with the JSON/YAML tags so that our CI can parse the output of the linter. I think this might be worth exporting to here if it will help you/others.

Worth mentioning that you can define these types yourself as well, they don't change really ever, and won't change in an incompatible way.

In our case, api-linter is one of many tools in ci pipelines and all the tools are executed with their defined response in json/yaml/errorformat, we need convert all the tools' response to a self-defined checkResult(maybe based on lsp), so that web or code review or other applications can use code check result in a standard format. My concern is that compatibility if we define structs with tag of api-linter, because we find that the struct Problem has been modified over time. Surely, we have defined struct Problem in practice, however, we suggest the api-linter add unmarshal of lint.Response, we think lots of teams or developers will benefit from this change.

BarryfwuTencent avatar Nov 02 '22 02:11 BarryfwuTencent