RefactoringEssentials
RefactoringEssentials copied to clipboard
icsharpcode
Inline method action
With regard to issue #134; inline method action. Created the inline method action for methods with simple complexity (1 line of code), unit tests for code coverage and unit tests for as many scenarios as I could think of.
Thx.
I am not certain I follow, please confirm my understanding.
Given a method to be inlined, M, defined within a document D1. An invocation of M by code within a different document, D2, will be incorrect or cause an exception.
If my understanding is correct, then I can say that I did in-fact create a unit test which asserts this exact situation in another branch. I removed the unit test in the final branch in which I submitted the pull request for 2 reasons; primarily because I added code which extended some core classes (non-breaking extension, but still extra-curricular) and felt it too risky for approval and second because I believed the test to be overly exhaustive.
If my understanding is correct, I will add the unit test into this branch and ensure that this does work as expected.
Thx!
When you collect locations of invocations of M, the locations will be related to the different documents (D1, D2). Now you use a method of the same SemanticModel instance on all locations. But a SemanticModel is always document-specific, so if you have a location in D2, you also will need the SemanticModel for D2.
Andreas, I added the test for refactoring across documents and a couple of supporting classes for such a test. Additionally, I found an additional case for delegate references, fixed the action and added tests for it.
Look forward to your feedback. Thx.
Hmm, just tried latest code with a simple usecase: I have an one-liner method in a class and call that method in another class (different .cs file). When I press Ctrl+. on this method's declaration, I still get an exception here: https://github.com/icsharpcode/RefactoringEssentials/pull/167/files#diff-dcd7a5a4e531ea43dba4e398edfd21a5R95
I was able to reproduce the problem. I now understand your original concern about using the SemanticModel's document and that was this problem. I changed to code to use what I believe is a much better method of using the caller and caller symbol. Also, stepping away and coming back, I found a few areas where I could reduce the likelihood of an exception occurring (more defensive checks) and changed those.
The multiple document test I created originally was flawed in that it had callers in both documents and managed to hide this problem (a consequence of me trying to test 2 concerns with a single test). I fixed this test so that it will not hide the problem and created an additional test for the other concern.
Thank you, looks great and now works over document borders! While testing I've found one problem in implementation: If replaced method is overloaded (means there is more than one with same name in document), an exception is thrown at https://github.com/icsharpcode/RefactoringEssentials/pull/167/files#diff-dcd7a5a4e531ea43dba4e398edfd21a5R252, because here you receive more than one symbol. Also searching only by method name might lead to removing of wrong method.
