D-Scanner
D-Scanner copied to clipboard
New analysis: Useless assignment
While trying to call a method/attribute of variable _fileSystem a segmentation fault will occur. There
is a subtitle bug during the assignment in the constructor. I wonder whether this could be found by D-Scanner
by throwing a warning for a self assignment _fileSystem = _fileSystem;?
class AppGenerator
{
private IfFileSystem _fileSystem;
this(IfFileSystem fileSystem)
{
_fileSystem = _fileSystem;
}
}
important: it should catch
class AppGenerator
{
private IfFileSystem fileSystem;
this(IfFileSystem fielSystem)
{
this.fileSystem = fileSystem;
}
}
To really work properly this would require D-Scanner to be a LOT more intelligent than it currently is.
doesn't D-Scanner already depend on dsymbol which would be most of the logic?
There are a few ways to go about this.
- Check to see if the AST of the left side is the same as the right. This is really easy and we can just look at the if-else-same check and use the same approach here.
- Use DSymbol to see if the left and right expressions resolve to the same symbol. This is a bit slower but will correctly handle some uses of
aliasandthis.. - Do some magic that will correctly handle things like
this.tupleof[0] = firstField;I'm not sure how to handle this best. I think that this will require an enhancement to DSymbol, as I don't think that it currently supports resolving symbols throughtupleof.
I would prefer proposal 2. It looks reasonable. If really needed, proposal 3 could be added in future on top.
Proposal 2 also wouldn't be that slow, it would just need to be checked if the identifier on the left side is the same as on the right side. I think supporting aliases or other confusing things would be overkill.
I started thinking about implementing this and realized that we'd need a way to provide import paths so that the DSymbol code can figure things out using imported modules. Then I found out that we already have that, but that it's not documented in the output of dscanner --help. https://github.com/dlang-community/D-Scanner/pull/812
There is also a pr to implement the check as dmd compiler warning https://github.com/dlang/dmd/pull/11553
It's probably a better idea to implement this in the compiler than in D-Scanner.
There is also a pr to implement the check as dmd compiler warning dlang/dmd#11553
Self-assignment of members in constructors will be an error. Others a warning.