- CodeQL overview
- Writing CodeQL queries
- CodeQL query help documentation »
- CodeQL query help for Go »
Useless assignment to field ¶
Click to see the query in the CodeQL repository
A value is assigned to a field, but its value is never read. This means that the assignment has no effect, and could indicate a logic error or incomplete code.
Recommendation ¶
Examine the assignment closely to determine whether it is redundant, or whether it is perhaps a symptom of another bug.
The following example shows a simple struct type wrapping an integer counter with a method reset that sets the counter to zero.
However, the receiver variable of reset is declared to be of type counter , not *counter , so the receiver value is passed into the method by value, not by reference. Consequently, the method does not actually mutate its receiver as intended.
To fix this, change the type of the receiver variable to *counter :
References ¶
Go Frequently Asked Questions: Should I define methods on values or pointers?
The Go Programming Language Specification: Method declarations .
Common Weakness Enumeration: CWE-563 .
Two linters I'll always add to new Go projects: errcheck and ineffassign
Jul 17, 2020
The reason for that is that they are the best bang for your buck by far. They are easy to appease, I have yet to see them produce false positives, they very frequently catch outright buggy code and retrofitting them is a pain.
ineffassign
This is the sort of thing that ineffassign prevents:
This is perfectly valid code, and will write garbage sometimes, without any indication whatsoever. Yes, Go does refuse to compile code like this
because data is not used, but declaring a variable and then immediately overwriting its value is perfectly legal and almost never what you want. At best the variable was never needed, in which case a _ is a good way to signal it, or it was meant to be used and ineffassign found a bug.
I’ve seen this pattern frequently enough in tests, where part of the test code accidentally doesn’t check intermediate errors or return values, leading to parts of the test silently breaking over time.
To its credit, gopls does check for this now .
In a similar vein, errcheck enforces checking error return values. In Go, errors are values, and unlike other 1 languages 2 , nothing enforces they are checked.
This is valid:
And so are all these:
The only difference is that the first example carries no indication where this was intentional or not. errcheck enforces that error values are at least assigned to _ , therefore being explicit that a decision was made to ignore the error 3 .
One case where this matters is when writing new code. It’s easy enough to forget about checking all error returns of all functions called. Again, tests passing by accident is a very frequent occasion, and so is production code.
Another also interesting case is functions changing signatures. When adding an error return to a function that previously returned nothing or updating a dependency that does so, you probably want to verify all the call sites, at the very least making the executive choice to explicitly ignore the errors.
Retrofitting using golangci-lint
golangci-lint has positioned itself as the tool everyone uses on CI, and I’d say with good reason. It supports many linters, has improved massively over the past couple of years and has facilities for wiring up into existing codebases by only checking code that changes 4 , allowing for incremental cleanup.
For example:
No one has to fix the unchecked error, until they touch the call in bar() . This works well, until you realise there are transformations where this heuristic falls flat. This is still true according to the latest golangci-lint, 1.28.3.
Here is an example of this in action:
Since the call to foo() is not touched, golangci-lint considers the unchecked error pre-existing and does not report it! The check is completely elided on changes that simply go from zero returns to a single error return. This simply makes the check not as useful as it could be, allowing regressions to merge over time.
The other problem with retrofitting is that the cleanup can be boring and take a long time. Clearing hundreds for errors in bulk is mind-numbing. Merely shifting around existing code might require fixing existing issues, unrelated to the change at hand.
Why go through that, when simply adding these linters from the start does the trick and saves you from bugs?
Addendum - 18th July
I got a bit curious about k8s’ code, and ran ineffassign against it. There is one case where ineffassign could be considered noisy, and that is using foo := true instead of var foo bool :
The code in question:
This nudges towards var exist bool or bool := false . Clearly there is no bug here, the result is the same either way, so it boils down to the style used when declaring variables.
good ↩
Rust ↩
Not necessarily a good decision, you can always find yourself staring at git blame wondering why. ↩
according to git and revgrep, using the new- settings in the config . Nowadays it works, a long time ago I found out the hard way it didn’t ↩
ineffassign
This package is not in the latest version of its module.
Detect ineffectual assignments in Go code.
This tool misses some cases because does not consider any type information in its analysis. (For example, assignments to struct fields are never marked as ineffectual.) It should, however, never give any false positives.
Documentation ¶
There is no documentation for this package.
Source Files ¶
- ineffassign.go
Keyboard shortcuts
DEV Community
Posted on Aug 11, 2022 • Updated on Aug 14, 2022
Go Method: Pointer Over Value Receiver Type
Avoid Using Value Receiver Type in Golang Structs Methods
About this article
This article assumes the readers already know basic knowledge of struct , methods , and goroutine in Go programming language.
The main purpose of this article is as reminder for myself about the use of value/pointer receiver type in struct methods (Go programming language). I decided to write about this, even though this is a very simple thing, after I spend more then 1 hour to debug an issue caused by the use of value receiver type in struct methods.
In an example below I will show the issue caused by the use of value receiver but without showing any warnings.
Receiver Types In Struct Methods
In Golang we define methods for a struct similar to methods in object-oriented programming language. The difference with OO language is in Golang there are 2 types of of receiver : value and pointer receiver types.
Pointer receiver type will copy the pointer of the 'object/struct' in memory. This is the same with methods in OO programming language where any changes made in the methods will change the actual 'object/struct' in memory. Example of pointer receiver type is as below :
Value receiver type will copy the 'object/struct' data. Any changes made to the object/struct' in methods will not change the actual 'object/struct' in memory. Example of value receiver type is below :
In Golang we only use the value receiver if the method do not need to make any changes to the struct data. However that is not true for all cases. In this article I will show an example where we need to use pointer receiver in a 'read-only' method.
Value Pointer Type
Example of ineffective assignment warning.
The Go language server (vscode extension), gopls , will show ineffective-assignment warning to the assignment in changeTitle and addABook methods. This is very helpful to prevent any bugs in case we forgot to use pointer receiver type.
Example in goroutine implementation
There is also other condition where pointer receiver is needed, but value receiver is used, yet no warnings are shown. This situation arise with the use of goroutine s, like in the code below :
Running code above will produce a false result. The Library in the timeConsumingOperation will have the outdated version (since the methods were run as goroutine ).
Pointer Receiver Type
The use of pointer receiver type in struct methods is mandatory if we want data integrity. Of course there are smaller number of cases where we need to use value receiver type. Modification of earlier example, with pointer receiver type on the timeConsumingOperation , can be seen below (added an asterisk(*)).
Using pointer type in timeConsumingOperation will points the lib to the same data in the memory, assuring data integrity. Running the modified code will produce :
Using pointer receiver in methods, instead of value receiver types, will assure data integrity. I suggest that we should , maybe except in few cases, to always use pointer receiver types to avoid potential bugs that are not catched by tools like the case above.
Here is a link of Code xample repository in Github.com .
Hope this article can be useful and thanks for reading
Top comments (0)
Templates let you quickly answer FAQs or store snippets for re-use.
Are you sure you want to hide this comment? It will become hidden in your post, but will still be visible via the comment's permalink .
Hide child comments as well
For further actions, you may consider blocking this person and/or reporting abuse
Building CRUD with ES6 JavaScript and Local Storage
Roselle Tabuena - Mar 17
What is Jenkins ?
vishva desai - Feb 26
Understand lifetime as a Rust beginner
Arc - Mar 17
Options Pattern em Go
Felipe - Mar 17
We're a place where coders share, stay up-to-date and grow their careers.
IMAGES
VIDEO
COMMENTS
@Ishmeet, note that the linter wasn't forcing you to use a var declaration; instead it hinted at that you might have a bug in your code—because the first assignment could be way more involved like assigning the return value of some function call which would return a value inintialized in a complicated way, and possibly depending on some input data.
ineffassign. Detect ineffectual assignments in Go code. An assignment is ineffectual if the variable assigned is not thereafter used. This tool misses some cases because it does not consider any type information in its analysis. For example, assignments to struct fields are never marked as ineffectual. It should, however, never give any false ...
1)ineffective assignment to field Hooks.OnNewEmail (SA4005)go-staticcheck 2)ineffective assignment to field Hooks.BeforeSendingEmail (SA4005)go-staticcheck 3)this value of err is never used (SA4006)go-staticcheck
ineffassign. Detect ineffectual assignments in Go code. An assignment is ineffectual if the variable assigned is not thereafter used. This tool misses some cases because it does not consider any type information in its analysis. For example, assignments to struct fields are never marked as ineffectual. It should, however, never give any false ...
This behavior is known as pass-by-value semantics and results in an "ineffective assignment to field" warning when attempting to update a struct field from within a method. The Solutions: Solution 1: Use a pointer receiver. To properly update the value of a field of a struct using a method, you should use a pointer receiver.
Hi, if it's possible, I would like to work on this. I had run ineffassign on x/text again an found more results. See the the updated list below. Almost all issues can be easily fixed by simply removing or refactoring the ineffectual assignments clauses.
The following example shows a simple struct type wrapping an integer counter with a method reset that sets the counter to zero. package main type counter struct { val int } func (c counter) reset() { c.val = 0 } However, the receiver variable of reset is declared to be of type counter, not *counter, so the receiver value is passed into the ...
The reason for that is that they are the best bang for your buck by far. They are easy to appease, I have yet to see them produce false positives, they very frequently catch outright buggy code and retrofitting them is a pain.
ineffective assignment to variable input (SA4005) Takeaways. When you pass a variable to a function, the function actually receives a copy of that variable's value. So if the function modifies the value, that change won't be reflected in the original variable.
ineffassign. Detect ineffectual assignments in Go code. This tool misses some cases because does not consider any type information in its analysis. (For example, assignments to struct fields are never marked as ineffectual.) It should, however, never give any false positives.
Go is a new language. Although it borrows ideas from existing languages, it has unusual properties that make effective Go programs different in character from programs written in its relatives. A straightforward translation of a C++ or Java program into Go is unlikely to produce a satisfactory result—Java programs are written in Java, not Go.
When you return, the changes made to the copy are lost. That's why you get a warning: you modify a field which you never use: you don't use in in the method after the assignment, and you can't possibly use it anywhere else, because after returning from the method, the effect of the assignment is lost.
Please note the usage of the assignment operator "=" instead of the short variable declaration operator ":=", as noted in this answer in SO. What was actually done was the assignment of the original "slice" values and "0" to the "newSlice" extra positions, but it was also a waste, as Go had already initialized them as "0" in the make() statement.
The Go language server (vscode extension), gopls, will show ineffective-assignment warning to the assignment in changeTitle and addABook methods. This is very helpful to prevent any bugs in case we forgot to use pointer receiver type. Example in goroutine implementation . There is also other condition where pointer receiver is needed, but value receiver is used, yet no warnings are shown.
I understand entirely the constraints on the compiler, but the problem is that the compiler is a likely home for all the newer analyses. What I imagine is that either go vet gains access to SSA-derived information, or else that we add a mode/flag to the compiler that would enable a bunch more warnings.
The reasons your 2nd short assignment gets through is because value2 is indeed new to "delcare and define". Go allows multiple short assignments only there is new variable. HOWEVER, the above is not applicable for assignment in reference (e.g. struct field). In that case, you need to declare the err variable first then assign accordingly ...
Saved searches Use saved searches to filter your results more quickly
I have a problem with structure fields. I've created a class Point with one method Move() that increases or decreases object variable x by dx.Another method Print is used to output results.. In main() a new instance is created with default x = 3 and dx = 2, then I call Move() and Print().I expect that value of x is changed during Move() and Print() will produce Final x=5, but instead of it ...
In 2018. And never remembered to fix the check. Let's use this issue as the "add back SA4005" issue. dominikh mentioned this issue on Jun 14, 2020. staticcheck: ineffective assignment SA4005 when modifying value receiver #141. Closed. dominikh closed this as completed in 7a27d6a on Apr 10, 2021.