Extract and reuse duplicated simplification warning #429
Conversation
|
I personally think that we should remove this tests. I thought it was a good idea when it was added, but I consider it to be dangerous nowadays: What if we remove simplifications? We would not detect that certain checks are unable to deal with the unsimplified code. Thus, we should be careful when writing the test code and should not rely on such automatic testing. Thus, I would vote for removing this simplification warning. |
|
I guess too much depends on simplifications and they are very unlikely to be removed. |
|
We will have to remove some to improve checking accuracy. |
That is a good point. However, without these warnings people misplace tests. Yes misplaced tests have the advantage that they test the result of the entire system. if the simplification is removed and this cause FN in a checker then with the misplaced test we would see it. Misplaced tests have the disadvantage imho that they make it hard to write a good test suite. Imho our tests are too much "just throw in various ad-hoc tests in random places" currently. I would really like if we had some more clever method for organizing and adding tests.
Yes. I want that we remove simplifications where checkers can use AST/ValueFlow instead. As you both say this is dangerous - we could easily introduce unseen FN. There are no proper tests currently. We need to be very careful. |
|
I applied these refactorings with 1e298a3 We can still remove the "unsimplified code warnings" later. I don't see how this refactorisation changes that. as far as I see this refactorisation even makes that job a little bit easier. My only real complaint about removing it, is that it causes misplaced tests and therefore less organization.. if we can do something to still get good enough organization then removing it is fine as far as I see. |
As we can control what we apply and what not, we should be able to keep it clean. |
3 participants
No description provided.