dotnet / codeformatter

Tool that uses Roslyn to automatically rewrite the source to follow our coding styles
MIT License
1.24k stars 244 forks source link

Reflection, semantic changes #240

Open tigerclawn82 opened 8 years ago

tigerclawn82 commented 8 years ago

Hi, i detected unusual behavior while testing code formatter.

Original Code

class A
        {
            public int F1;
            private int F2;
            protected int F3;
            public static int SF;

            public int P1 { get; set; }
            private int P2 { get; set; }
            static int SP { get; set; }

            public A()
            {
                F2 = 2;
                F3 = 3;
            }
        }

        class B
        {
            public int F1;
            public int F2;
            public int F3;
            public static int SF;

            public int P1 { get; set; }
            public int P2 { get; set; }
            public int P3 { get; set; }
            static int SP { get; set; }
        }

        [Fact]
        public void Test_Extensions_Generic_Map()
        {
            var A = new A
            {
                F1 = 1,
                P1 = 1
            };
            A.SF = 4;
            var B = A.Map<A, B>();
            Assert.Equal(1, B.F1);
            Assert.Equal(2, B.F2);
            Assert.Equal(3, B.F3);
            Assert.Equal(4, B.SF);
            Assert.Equal(1, B.P1);
            Assert.Equal(0, B.P2);
            Assert.Equal(0, B.P3);

            B = new B
            {
                F1 = 2,
                F2 = 1
            };
            B.Map(A, "F1", "p1");
            Assert.Equal(2, B.F2);
            Assert.Equal(3, B.F3);
            Assert.Equal(4, B.SF);
            Assert.Equal(0, B.P1);
            Assert.Equal(0, B.P2);
            Assert.Equal(0, B.P3);
        }

Formatted Code

private class A
        {
            public int F1;
            private int _F2;
            protected int F3;
            public static int SF;

            public int P1 { get; set; }
            private int P2 { get; set; }
            private static int SP { get; set; }

            public A()
            {
                _F2 = 2;
                F3 = 3;
            }
        }

        private class B
        {
            public int F1;
            public int F2;
            public int F3;
            public static int SF;

            public int P1 { get; set; }
            public int P2 { get; set; }
            public int P3 { get; set; }
            private static int SP { get; set; }
        }

        [Fact]
        public void Test_Extensions_Generic_Map()
        {
            var A = new A
            {
                F1 = 1,
                P1 = 1
            };
            A.SF = 4;
            var B = A.Map<A, B>();
            Assert.Equal(1, B.F1);
            Assert.Equal(2, B.F2);
            Assert.Equal(3, B.F3);
            Assert.Equal(4, B.SF);
            Assert.Equal(1, B.P1);
            Assert.Equal(0, B.P2);
            Assert.Equal(0, B.P3);

            B = new B
            {
                F1 = 2,
                F2 = 1
            };
            B.Map(A, "F1", "p1");
            Assert.Equal(2, B.F2);
            Assert.Equal(3, B.F3);
            Assert.Equal(4, B.SF);
            Assert.Equal(0, B.P1);
            Assert.Equal(0, B.P2);
            Assert.Equal(0, B.P3);
        }

Basically code semantic been changed, reflection based usage will break things up. Is there anything that can be configured to skip this kinda behavior?

Thanks

jaredpar commented 8 years ago

Which semantics were changed that are concerning? Is it the renaming of private fields?

tigerclawn82 commented 8 years ago

yes, renaming of private fields.

Assert.Equal(2, B.F2);

it will fail after formatting in-fact i came to know about it due to the test case fail.

jaredpar commented 8 years ago

We definitely understand this particular part of the code formatting can break reflection and binary serialization. In general that doesn't have an impact on code bases. Roslyn for instance converted ~150 projects to this rule with almost no issue. It was implemented because that is the desired standard for the dotnet organization.

That being said it's possible to disable this rule (just as with any other rule). Hence you can run everything else but this rule without issue.