go/token: MergeLine(0) should be a no-op, not a panic #9921
Comments
|
Calling MergeLine on a line derived from token.NoPos looks like a programmer error to me, and thus a panic seems appropriate. @griesemer what do you think? If you want it not to panic, I'll send a CL. |
|
I am also leaning towards leaving this as is. Note that if x.Pos() returns token.NoPos, then fset.File(x.Pos()) returns nil as well. It's not clear how MergeLine should be changed: I don't think we want to silently do nothing if the *File receiver is nil as that would hide errors. So we could only silently return if both the *File receiver is nil and the line number is 0. But that seems odd, too. Please re-open if you have strong counterarguments. I don't know that MergeLine is used all that frequently, and having callers making sure that the incoming *File and line are correct seems ok. |
This seems to be an idiom (found in ast.SortImports):
If x.Pos returns token.NoPos (0), then fset.Position(x.Pos()).Line == 0, and MergeLine panics. Instead of panicking I believe MergeLine should just return silently. This will make code doing things like this much more robust. Otherwise every call site has to guard against x.Pos() == 0.
The text was updated successfully, but these errors were encountered: