Should vet report usage of deprecated features?

[jasdel]

Since a standardized[1] deprecation syntax has been proposed, and accepted (i think?) to Go documentation, does it make sense to update vet to start reporting usage of deprecated values? I could see this as a useful update where it makes it easier for library users to easily identify they are using deprecated features. Having the annotation in documentation is useful, but can easily be missed.

[1] http://blog.golang.org/godoc-documenting-go-code

[Andrew Gerrand]

I wonder what the cost of doing this is.

On 16 March 2016 at 10:29, jasdel <delp...@gmail.com> wrote:

Since a standardized[1] deprecation syntax has been proposed, and accepted (i think?) to Go documentation, does it make sense to update vet to start reporting usage of deprecated values? I could see this as a useful update where it makes it easier for library users to easily identify they are using deprecated features. Having the annotation in documentation is useful, but can easily be missed.

[1] http://blog.golang.org/godoc-documenting-go-code

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Russ Cox]

On Tue, Mar 15, 2016 at 7:29 PM jasdel <delp...@gmail.com> wrote:

Since a standardized[1] deprecation syntax has been proposed, and accepted (i think?) to Go documentation, does it make sense to update vet to start reporting usage of deprecated values? I could see this as a useful update where it makes it easier for library users to easily identify they are using deprecated features. Having the annotation in documentation is useful, but can easily be missed.

At first glance, it seems to me that vet should not report usage of deprecated APIs. Due to Go 1 compatibility, deprecated is still supported. The criteria for vet are now documented at http://tip.golang.org/src/cmd/vet/README, and I think reporting usage of deprecated APIs fails both the correctness test (the code is likely still correct as is) and the precision test (code may well be using the old API for compatibility with older versions of Go, in which case the report is effectively a false positive).

Russ

[Andrew Gerrand]

It might be a better fit for golint?

--

[Rob Pike]

Yes, sounds to me like a lint issue.

-rob

[jasdel]

Thanks for the link and information on detailing vet's purpose. It makes a lot of sense for deprecation to not be in vet, and added to lint instead. I would be glad to help/implement this change.

Being able to easily identify if my code is using types/values which are deprecated would be helpful. Such as in cases where code checks for specific return value types which are no longer returned. Like those deprecated in compress/flate. Below is a rough idea of how this could be accomplished.

Output:

Using compress/flate's ReadError as an example lint could output an error like:

gopher.go:123:1: use of deprecated type flate.ReadError, No longer returned.

If lint detects something that is deprecated the text following the "Deprecated:" would be appended to the end of lint's message following the thing that was deprecated. Should the text grabbed be limited to the current line, or continue until a paragraph break in the comment text?

How:

An additional lint task could be added to lint to walk a file's content inspecting the packages, funcs, types, fields/ and methods used. When a deprecated thing is found a lint error will be added to the errors to report.

The "Deprecated" tag and its message probably would need to be extracted for the ast's comment group for the thing. Would need to determine how much text to copy from the comment group. What if a library has something odd like struct field deprecated both on the preceding line and trailing white space with different messages?

This probably would make sense to be at least two lint tasks, package and func/type/fields/methods to simplify the implementation code.

Any comments, suggests, or feedback would be great.

Cheers,

Jason

[David Symonds]

I don't see this as being suited to golint. Golint isn't a general
purpose code checking thing; it is for checking style, and using a
deprecated API isn't bad style. It's closer to bad practice. Some
other tool would be the place for it.

[Rob Pike]

I disagree. There is little meaningful difference between bad style and bad practice.

-rob

[Damian Gryski]

I thought since golint runs a single file at a time, there were certain classes of checks it couldn't do.  It seems to me that resolving the destination of a call and checking the associated source code comment would fall into that category.

Damian

[jasdel]

I'd like to dig into this problem a bit more. And maybe figure out what the cost of adding this feature would be. From a high level lint uses the ast on each package and splits the logic per file. With the ast per file. With I think the question is will lint have enough information about the ast being walked to determine if a thing is deprecated. Or are additional tools needed to be able to introspect the dependency thing being used?

Even though lint is operating on a per file with in a package I'm not sure if there is anything inheritantly preventing deeper introspections other than a more complex implementation. But I could easily be wrong here since I only have a passing experience with the ast.

Cheers,
Jason