Hacker News Reader

Comment by rickhanlonii | Hacker News Reader

Really neat package here.

Don't let me take away from the work that's been done here (and correct me if I'm wrong) but I think this assumes that the most generic type is the best option?

Personally, I don't think that's a correct assumption.

When you write a method header and define the argument types, you're writing the contract for that method. Sure, the particular implementation may only need to be io.Reader, but you don't code contracts for the implementation, you code them for what makes sense in your interface. For a lot of perfectly justified reasons you may choose a specific type over a more generic type.

So I see a utility in this for inspecting possible places to be more generic. But I would worry that someone would add this to, say, a git hook as way to enforce this assumption.

View on HN

To add to what jerf already said - This is a linter, so it should be taken with a grain of salt.

Having said that though, I'm trying really hard to make it conservative and not have any false positives. I always discard functions that may be implementing interface methods or declared function types, for example.

Is this what you meant by a contract? If so, interfacer would skip the function, even if nowhere in the code is the function used to implement the interface. It's just lazy and assumes a conservative position.

If you mean it in a broader way that might not involve any interface or named function type, then we need a way for the developer to somehow express in the code that they really want that specific type. For example, I made it so that mentioned parameter types are ignored:

  func ProcessInputFile(f *os.File) error {
      // use as an io.Reader
  }

This will not produce a suggestion.

I really want to make this tool as conservative and less noisy as possible, so if you have any ideas or a false positive example they would be very welcome.

> But I would worry that someone would add this to, say, a git hook as way to enforce this assumption.

You shouldn't. At that point there are so many more concerns I'd have with the rest of the code (less 0 side-effect usage).

"I think this assumes that the most generic type is the best option? Personally, I don't think that's a correct assumption."

Well, of course it isn't universally. But it's helpful to be able to scan something and get prompted that you made something an excessively-specific type for what is, at this time, no apparent reason.

One generic reason to use a specific type (heh) is to avoid resolving the interface on every use. But in general there's still a lot of code written that takes a concrete type but just needs a Reader/Writer/etc.

"But I would worry that someone would add this to, say, a git hook as way to enforce this assumption."

I suspect anyone using this is not likely to be tempted that way. There's a lot of linters/code analyzers for Go that are, like this, useful for feedback but can't be trusted to that degree. See https://github.com/alecthomas/gometalinter. I've got a profile of a gometalinter setup that I've got some of my projects hooked up to a pre-commit hook on [1], but only for selected hooks. Things supported in that package range from "really no good reason ever to have this 'linter' complain" to "I've hardly ever seen it emit a warning I agreed with" (sorry, gocyclo, but along with my general distaste for cyclomatic complexity being a useful metric, you're set too painfully low for me).

See also the other story on the HN front page as I write this: https://news.ycombinator.com/item?id=11396477 You would not hook that to a precommit hook. You'd just run it every so often.

[1]: If you're interested in this yourself, basically I just wrapped the metalinter behind a shell script, started committing, and whenever it complains about something I don't like, I either hack out the specific warning or the entire linter in question, depending. Best just to grow your own profile. You may also find this trick amusing: In the pre-commit hook, I actually have it chdir into the project root and run this:

    ./gml $(find -name lintclean -printf "%h ")

"gml" is the aforementioned shell script consisting basically of "gometalinter $ALL_THE_EXCLUSIONS $*", and the find comand makes it so you drop a file named "lintclean" in a package that you want to keep clean. The precommit hook then only triggers on packages that you've "declared" to be sealed to be clean, meaning you can develop freely without the linter screaming continuously.

> One generic reason to use a specific type (heh) is to avoid resolving the interface on every use.

Aren't Go interface checked at compile time? I thought only interface casting happened at runtime.

Casting one interface type to another involves a dynamic lookup, since you don't know what concrete type you're going to have at runtime. See http://research.swtch.com/interfaces for context.

This might be relevant: https://github.com/mvdan/interfacer/issues/19

And thanks for the link! That should be useful to make a decision on what to do in that issue.

Checking that a value put into an interface type conforms to the interface type is done at compile time, so modulo issues with "nil" you can't have an interface type where the wrapped type will turn out not to have the desired method at run time. However all method invocations on that interface will pass through a dynamic lookup, like going through a vtable in C++. It's one of the things that makes the people who are very, very serious about absolute speed not use Go.

I imagine it also prevents inlining in most or all situations. (I don't know exactly how hard the compiler will try to notice that something is nominally in an interface value but it can be sure it is actually a concrete type. My guess is "not at all".) Given that inlining is Go's big optimization technique, that can be another generic reason to use a specific type instead of an interface.

Reply on news.ycombinator.com