RefactoringEssentials
RefactoringEssentials copied to clipboard
Fixes #301. Fix up ConvertAutoPropertyToPropertyTest
Hi There
This PR only fixes one test, but I though it would be good to discuss some things now. This should avoid the situation where I spend a lot of time fixing up, but I do it in a way that you are unhappy with and then I have to do it again!
I added codescene to the readme for my own benefit, and have just noticed that this made its way in to this commit. I'm happy to take it out again if you would like.
I have done a bigger job than is necessary fixing up ConvertAutoPropertyToPropertyTest, and this is mainly so that we can talk through the kinds of changes I might make later.
CodeRefactoringStatementSyntax is a wrapper around the Roslyn StatementSyntax that allows me to write fluent syntax. If you ask me to remove this class I wouldn't be too offended.
DocumentManipulator is a base class that allows me to group related code in the same place and avoid the passing of some parameters and other such things. It has a SystemDot property, which is the bit that works around the Roslyn simplifier bug (that used to simplify System.NotImplementedException to NotImplementedException if the using statement exists, but no longer does). I am quite attached to this class, as I think I can group all the Roslyn workarounds in it, which will make it easy to remove them once the Roslyn bugs are fixed, or to add new workarounds if there are other problems.
ConvertAutoPropertyToPropertyManipulater descends from DocumentManipulator. It is a part of the plan to group all the Roslyn workarounds together, and is quite a neat singly responsible class in my opinion. It also helps cut down on parameter use / passing.
PropertyDeclarationContext and PropertyDeclarationContextFinder encapsulate a useful bit of code in my opinion, and will be used by other refactorings. They are incidental to the fixing of this test.
There was a typo in the test (using Sytem instead of using System), which I initially thought was the problem (as opposed to the bug in Roslyn), but I checked for this and the Rosyln problem remained after I fixed the typo.
Cheers
Cedd