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

Document output schemas #322

Closed
TheRealAgentK opened this issue Jul 1, 2017 · 21 comments
Closed

Document output schemas #322

TheRealAgentK opened this issue Jul 1, 2017 · 21 comments

Comments

@TheRealAgentK
Copy link
Collaborator

TheRealAgentK commented Jul 1, 2017

Please document the different output schemas (e.g. JSON, XML, text)
(was part of #318 from @KamasamaK - created a separate issue)

@TheRealAgentK
Copy link
Collaborator Author

Also:

"Also, curious when the locations array would contain anything but one element."

"I don't want my second question to get lost, though. Is it possible, and if so in what cases would the locations array not contain one element? Likewise for the XML output, as that would be important to define the minOccurs and maxOccurs attributes in a schema as well as a description of what the element is."

@TheRealAgentK
Copy link
Collaborator Author

TheRealAgentK commented Jul 1, 2017

I had a look into the location structure.

In XML it seems that it's possible to get multiple location elements for an issue. The code is:

do {
                    writer.append("<location");
                  ...
                    writer.append(">");
                    writer.append("<Expression><![CDATA[")
                   ...
                            .append("]]></Expression>");
                    writer.append("</location>").append(System.getProperty(LINE_SEPARATOR));
                    prevbugInfo = bugInfo;
                    bugInfo = iterator.hasNext() ? iterator.next() : null;
                } while (isGrouped(prevbugInfo, bugInfo));

(the JSON code is similar)

So, isGrouped() decides if there's one or multiple location elements. I'm not sure why and how exactly that's possible - in everything I've done with CFLint so far I've only seen single location elements in the XML. It seems to me that isGrouped is always false in my applications

I think @ryaneberly might have to have to look into that, sorry.

cc: @KamasamaK

@TheRealAgentK
Copy link
Collaborator Author

Discussion in #318 leaning towards not touching locations right now, but look into it again for 2.0

@TheRealAgentK
Copy link
Collaborator Author

@KamasamaK Just to let you know, we've done #323 and #330 - therefore you'll find that CFLint XML, Json and Text output now have a few more counters/stats. Findbugs hasn't been changed.

(all in dev branch, which we'll release 1.2.0 from in the next few days hopefully)

@bardware
Copy link

bardware commented Jul 2, 2017

"Also, curious when the locations array would contain anything but one element."

I am under the impression after fixing this issue location would contain either a struct or an array. Is this the case? Are you going to make sure location will always contain an array?

@TheRealAgentK
Copy link
Collaborator Author

@bardware locations (it was changed to plural in #158) is now always a JSON array, yes. It can conceptually contain 1-many objects, but in practice seems to have only one object for the time being.

@KamasamaK
Copy link
Collaborator

I recommend using JSON Schema Store for the JSON schema.

@TheRealAgentK
Copy link
Collaborator Author

@KamasamaK Sure - it'd be great if you could create a JSON Schema for us to test. When it's all working, more than happy to send a Pull Request to Schema Store with it (or you can do it yourself then).

@KamasamaK
Copy link
Collaborator

@TheRealAgentK I'll give it a shot, but I will have to infer some of the meanings. For example, the top-level message is always the same as the code. I assume the intent was to eventually change it since it doesn't make sense that way, but I will have to take it at face value and just describe them as being the same for now.

I've changed my mind about using JSON Schema Store for the output right now. This is due to any potential breaking changes as a result of Issue #331. Breaking code compatibility is one thing, but I would want to minimize the possibility of having cflint-result.json and cflint-result-v2.json schemas hosted there.

@TheRealAgentK
Copy link
Collaborator Author

I'm not sure if #331 would necessarily change the output formats.

I created the ticket to refactor the internal workings of how CFLint produces the various output formats because it's currently using different ways to get to the same point.

@TheRealAgentK
Copy link
Collaborator Author

@KamasamaK Re your other question (top-level message vs. top-level code) --- I'm not sure what the intention was when it was built. Maybe @ryaneberly can shed some light into it?

Regardless though - there's no reason we can't change it. For instance in findbugs XML, I don't do that, the built-in CFLint XML currently has it though.

So, in hindsight - maybe #331 WILL change a bunch of things in the output formats :-)

@KamasamaK
Copy link
Collaborator

@TheRealAgentK You're right, it does say it's specific to the engine. Although I think consistency of the data should also be a goal (and a consequence) of having more cohesive serialization.

For example, the text output has Severity, Message code, File, Column, Line, Message, Variable, and Expression whereas the JSON/XML output has severity, id, message, category, abbrev, location.file, location.fileName, location.function, location.column, location.line, location.message, location.variable, and location.expression.

Maybe the goal isn't to make them identical, but I think at least naming should be consistent. It's strange to have a field in the text output be Message code and the same data is referred to as id and message in JSON/XML.

@TheRealAgentK
Copy link
Collaborator Author

Yeah, I agree - they should be consistent and we'll make sure of that when I do #331 then at some point.

I'd still think it'd be useful to document the current "schemas" as of now and provide that info to people.

@KamasamaK
Copy link
Collaborator

KamasamaK commented Jul 24, 2017

I have the XML and JSON schemas finished for the v1.1.0 formats. I notice that the output of the dev branch changes this and I will need to modify the schemas and my plugin if this format is final. The plugin I was developing was assuming that each JSON object in the array would have the same structure. I will bring up my concerns with that separately in Issue #331. I would like to ask here, though, if those changes are intended for v1.2.0 or later.

@TheRealAgentK
Copy link
Collaborator Author

Will change JSON (see #323), CFLint XML only had some minor count and timestamp additions in 1.2.0 and Findbugs is using the proper bugcollection.xsd anyway.

@KamasamaK
Copy link
Collaborator

KamasamaK commented Jul 28, 2017

I've put schemas for the JSON and XML in /src/main/resources/schemas/ in PR #377

@KamasamaK KamasamaK self-assigned this Jul 28, 2017
@KamasamaK
Copy link
Collaborator

Update to match change in Issue #383

@TheRealAgentK
Copy link
Collaborator Author

@KamasamaK just saw this now - you hadn't updated the schema yet in the repo though? I just changed timestamp to type number and pushed/merged it in my PR.

@KamasamaK
Copy link
Collaborator

It's done now. I'm not sure how to create a proper schema for the text output, though. Any suggestions?

Otherwise, I guess this issue can be closed unless someone finds something to change with the current result schemas. But really, the schemas will always be a work-in-progress to match changes with the generated result formats.

@TheRealAgentK
Copy link
Collaborator Author

@KamasamaK Yeah, to be frank, the text output is just what it is. I don't think - similar to the HTML report - that it'd need a schema or any detailed documentation.

@KamasamaK
Copy link
Collaborator

HTML would be too complicated, I think. I will probably add something for the text as a code block in the README.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants