r297034 - [clang-format] Add tests for ambiguous namespaces to the comment fixer

[Daniel Jasper via cfe-commits]

On Mon, Mar 6, 2017 at 6:29 PM, Krasimir Georgiev via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: krasimir
> Date: Mon Mar  6 11:29:25 2017
> New Revision: 297034
>
> URL: http://llvm.org/viewvc/llvm-project?rev=297034&view=rev
> Log:
> [clang-format] Add tests for ambiguous namespaces to the comment fixer
>
> Modified:
>     cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp
>
> Modified: cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/
> NamespaceEndCommentsFixerTest.cpp?rev=297034&r1=297033&r2=297034&view=diff
> ============================================================
> ==================
> --- cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp
> (original)
> +++ cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp Mon Mar
> 6 11:29:25 2017
> @@ -406,6 +406,86 @@ TEST_F(NamespaceEndCommentsFixerTest,
>                                      "#else\n"
>                                      "  int j;\n"
>                                      "#endif"));
> +  EXPECT_EQ("#if A\n"
> +            "namespace A {\n"
> +            "#else\n"
> +            "namespace B {\n"
> +            "#endif\n"
> +            "int i;\n"
> +            "int j;\n"
> +            "}",
> +            fixNamespaceEndComments("#if A\n"
> +                                    "namespace A {\n"
> +                                    "#else\n"
> +                                    "namespace B {\n"
> +                                    "#endif\n"
> +                                    "int i;\n"
> +                                    "int j;\n"
> +                                    "}"));
>

I don't understand why we aren't adding "// namespace A" here. I mean I
don't think it matters much, but it seems weird to me.


> +  EXPECT_EQ("#if A\n"
> +            "namespace A {\n"
> +            "#else\n"
> +            "namespace B {\n"
> +            "#endif\n"
> +            "int i;\n"
> +            "int j;\n"
> +            "} // namespace A",
> +            fixNamespaceEndComments("#if A\n"
> +                                    "namespace A {\n"
> +                                    "#else\n"
> +                                    "namespace B {\n"
> +                                    "#endif\n"
> +                                    "int i;\n"
> +                                    "int j;\n"
> +                                    "} // namespace A"));
>

I'd consider merging this test with the previous one. Might make it more
obvious what the difference is. But I am not sure.


> +  EXPECT_EQ("#if A\n"
> +            "namespace A {\n"
> +            "#else\n"
> +            "namespace B {\n"
> +            "#endif\n"
> +            "int i;\n"
> +            "int j;\n"
> +            "} // namespace B",
> +            fixNamespaceEndComments("#if A\n"
> +                                    "namespace A {\n"
> +                                    "#else\n"
> +                                    "namespace B {\n"
> +                                    "#endif\n"
> +                                    "int i;\n"
> +                                    "int j;\n"
> +                                    "} // namespace B"));
> +  EXPECT_EQ("namespace A\n"
> +            "int i;\n"
> +            "int j;\n"
> +            "#if A\n"
> +            "}\n"
> +            "#else\n"
> +            "}\n"
> +            "#endif",
> +            fixNamespaceEndComments("namespace A\n"
> +                                    "int i;\n"
> +                                    "int j;\n"
> +                                    "#if A\n"
> +                                    "}\n"
> +                                    "#else\n"
> +                                    "}\n"
> +                                    "#endif"));
>

Similarly, I would have expected us to insert "//namespace A" here. Why
don't we. Is that related to the missing "{" after "namespace A"?


> +  EXPECT_EQ("namespace A\n"
> +            "int i;\n"
> +            "int j;\n"
> +            "#if A\n"
> +            "} // namespace A\n"
> +            "#else\n"
> +            "} // namespace A\n"
> +            "#endif",
> +            fixNamespaceEndComments("namespace A\n"
> +                                    "int i;\n"
> +                                    "int j;\n"
> +                                    "#if A\n"
> +                                    "} // namespace A\n"
> +                                    "#else\n"
> +                                    "} // namespace A\n"
> +                                    "#endif"));
>  }
>
>  TEST_F(NamespaceEndCommentsFixerTest,
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170306/e67fbea6/attachment-0001.html>