Inline method action

[cstick]

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.

[cstick]

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!

[Rpinski]

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.

[Rpinski]

Means: Yes, such a test would be important :-)

[cstick]

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.

[Rpinski]

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

[cstick]

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.

[Rpinski]

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.

[siegfriedpammer]

Any updates? Closable?