// Compact appends to dst the JSON-encoded src with
// insignificant space characters elided.
After reading the relevant code sections, It appears Compact compacts as well as validates json. Calling json.Valid after calling json.Compact would be redundant. I think the docs should help make that clear.
The only reason I can think of that change not being accepted is if json.Compact isn’t guaranteed to validate. If the reference implementation of Go validates, that’s a nice-to-have, but if they add it to the documentation, that impliew that alternative implementations have to validate.
Compact appends to dst the JSON-encoded src with insignificant space characters elided.
Compact appends JSON-encoded src, where JSON encoding is defined in RFC 7159. If src is not JSON-encoded, then src is not appended, and a non-nil error is returned.
Since compact returns an error, I think valid json is implied. It would be nice to have this guarantee explicitly documented.
Thanks to petrus’s input, I think this is what it should read:
// Compact appends to dst the JSON-encoded src with insignificant space
// characters elided. If src is not valid JSON, src is not appended and a non-nil
// error is returned.
That sounds to me like a coincidence. Of course, you can suggest it as a PR or issue on GitHub, but they may say that Compact isn’t guaranteed to validate. Perhaps the implementation in the future won’t step through the JSON state machine and report an error.
I don’t know if the Go standard library works this way, but in C++, the standard library describes the prerequisites and results of its functions. There is a reference implementation, but compilers and 3rd parties can implement their own standard libraries where they must meet the minimum requirements described by the standard. The Go documentation defines Compact as removing unnecessary whitespace. The mechanism that the Go reference implementation uses will detect errors just like the Validate function, but that might be a pleasant side effect of the implementation. If it’s documented as validating the JSON, then all custom implementations will have to validate the JSON, too.
It might turn out that someone comes up with a super-fast method of compacting JSON without the JSON state machine, but if the documentation says the JSON is validated as it’s compacted, then it has to be valid, because people will rely on it. The implementer of this super-fast Compact implementation will then have to add a call to Valid which might slow the implementation down, even though whoever called Compact might not need that guarantee.
Documentation is the “contract” describing functions’ pre- and post-conditions. Code is just the implementation.
I agree with what you are saying especially with documentation being the contract. This is my concern with documenting that the function performs validation. However, I think leaving the function ambiguous is also not desirable.
The question remains, what is error if not for invalid JSON?
So perhaps would it be better to document, “For this implementation of Compact, if src is not valid JSON, src is not appended and a non-nil.” Alternatively, “Future versions Compact may not have this guarantee.” I think the latter would be my preference.
So this is my updated proposal:
// Compact appends to dst the JSON-encoded src with insignificant space
// characters elided. If src is not valid JSON, src is not appended and a non-nil
// error is returned. Future versions Compact may not have this guarantee.
I apologize for the back and forth; I believe that I am not explaining myself clearly. My point is that this ambiguity is good.
If I had no access to the source code and saw that the description of the function was:
Compact appends to dst the JSON-encoded src with insignificant space characters elided.
I would expect nothing more and nothing less.
If the implementation failed to omit insignificant whitespace in certain situations, I would consider that a bug and open an issue.
If the implementation was too slow because it’s running the full JSON state machine and if there was some hypothetical algorithm that omitted whitespace more efficiently without validating it, then that would be a bug and I would also open an issue.
I believe that the fact that the Go reference implementation’s Compact function runs the JSON state machine to determine the validity of the compacted JSON is an implementation detail. It’s nice that it does it, but, based on the documentation, it is not required.
My understanding of standard libraries, in general, is that you could copy and paste the documentation of the Go standard library reference implementation over some other implementation that might work completely differently and still be valid. Programs compiled with this non-standard implementation would also still be valid. It is an implementation detail that Compact checks if the JSON is valid. If you recompile, for example, with gccgo, perhaps that implemetation of json.Compact would not validate the JSON.
Good question, and one that may, indeed, quell my concerns, depending on the answer from the Go team and/or community, should you open an issue.
Based on the documentation, alone, I believe that is intentionally opaque. Looking at (*bytes.Buffer).Write, perhaps json.Compact will report ErrTooLarge if enough memory for the JSON cannot be allocated in the *bytes.Buffer. With the current reference implementation, perhaps the only error returned will be from invalid JSON, but by returning error, the documentation opens up the possibly returned error results to alternative implementations.
I, personally, disapprove of this kind of documentation because it’s too irresolute. Part of what makes Go programmers productive is, frankly, the plainness of the language and its standard library. When I read a function’s documentation, it explains what happens to the inputs and outputs. I don’t want to use methods of a package that says the current implementation has some behavior but future implementations may have some other behavior.
P.S.
I’d like to say that I am not familiar with the Go team’s policies and that my opinion could be completely against what the Go team is for. I could be very wrong! If you believe that the documentation should be updated, by all means, submit an issue to the Go team. I do not mean to be any sort of Internet troll that declines any ideas about changes to the Go language or anything about it. I’m just trying to respond to the initial suggestion of changing the documentation to include that the Compact function also validates the JSON being compacted. In my opinion, based on the information I have in my head right now, I don’t think the documentation should be changed unless careful consideration is given to non-standard implementations of the Go standard library (and, perhaps there has been).