Closed faceair closed 1 year ago
@faceair Thanks for your feedback. We will disable this lint by default in the next version.
To help fix it, do you use any build tags?
I am using arenas, so there is an environment variable GOEXPERIMENT=arenas, without any other tags.
@faceair Ok. Then, it's not obvious what it might be. I have seen some problems like this with build tags (though, I don't have a good solution to support multiple tags at the same time and resolve all references, and be performant).
The variable c is an interface that may have two implementations, but the methods of this DataType will always return a value of the DataType type. Not sure if this information is helpful to you.
@faceair It's very helpful! I thought c is a package import.
Added 2 new settings: tooltitude.brokenRefsEnabled, and tooltitude.deprRefsEnabled, the first one will disable the error you have. The fix will be available closer to the end of this week in 0.14.0, most likely on Friday.
Thank you so much for fixing the bug so quickly! Keep up the great work!
I tried reproducing it. I created a project with 2 package. abc and c
package c
import "github.com/tooltitude/sandbox/abc"
func DoStuff2(c abc.Xyz) {
c.DataType()
}
func DoStuff() {
println("I did Stuff!")
}
package abc
type DataType uint8
type Xyz interface {
DataType() DataType
}
In DoStuff, DataType resolves well. Is your example similar to it, or is there any substantial difference? Do you use generics anywhere?
I double-checked and your lint is also correct. This "c" only has two possible implementations, so the default branch is actually unreachable. This code will not produce any errors during compilation, but it seems that your lint is stricter.
@faceair No, it doesn't take the control flow into account. Now, I see what's going on. It has default branch, and it doesn't assign any type to c, as far as I understand. I believe, I will be able to fix the issue now.
Was able to completely reproduce your example:
@faceair Thanks a lot for all helpful details!
@faceair Ok. Fixed it as well. Fix will be available in 0.14.0 as described previously. Will close this issue after that release.
Also, since we have the reason for this issue fixed, I will keep this inspection on by default.
I have another question. Actually, gopls supports shadown checking, but it is disabled by default. Does Tooltitude no longer need to provide this feature?
In fact, gopls has some very useful checkers that are also disabled by default. I have enabled these, for reference only.
"gopls": {
"ui.diagnostic.analyses": {
"nilness": true,
"unusedvariable": true,
"unusedwrite": true,
"useany": true
},
"ui.diagnostic.staticcheck": true,
"ui.semanticTokens": true
}
@faceair They are a bit different. The last time I checked, gopls, didn't check for package name conflicts while tooltitude does it.
@faceair I tried it right now, and found one more difference (in addition to package shadows not displayed): gopls one doesn't work on code with errors, i.e. the example from the documentation doesn't show shadows if the variables aren't resolved.
P.S. Concerning our approach to inspections, we aim to have a set inspections which are applicable to almost anyone writing go code as opposed to the ability to choose the one which you want to enable.
One more difference is that we don't report variables with name err, ok, and _ since they are shadowed so often that it's basically useless to warn about them.
@faceair v0.14.0 is out. Closing this issue. Please reopen it if you have it again.
Tooltitude Version: v0.13.3
OS Version: macOS 13.3.1
VS Code Version: 1.77.2
Go Version: 1.20.3
Code Repository (if open source): no
What did you try to do?
Is there anything interesting in the tooltitude output channel? (should be open if you are reporting error from within VS Code) no
My code is error-free, it compiles and runs without any issues. However, Tooltitude is giving me a strange error message and I have no idea what's causing it. There doesn't seem to be an option to disable it either.
I would like to: