[PATCH] D140274: [clangd] Tweak to fill struct with designated initializers
tupos created this revision.
tupos added reviewers: sammccall, njames93.
tupos added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
tupos requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D140274
Files:
clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
clang-tools-extra/clangd/refactor/tweaks/FillStructDesignatedInit.cpp
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/tweaks/FillStructDesignatedInitTests.cpp
Index: clang-tools-extra/clangd/unittests/tweaks/FillStructDesignatedInitTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/tweaks/FillStructDesignatedInitTests.cpp
@@ -0,0 +1,126 @@
+//===-- FillStructDesignatedInitTests.cpp ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TweakTesting.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TWEAK_TEST(FillStructDesignatedInit);
+
+TEST_F(FillStructDesignatedInitTest, AvailableUnavailable) {
+ Header = R"cpp(
+namespace ns1 {
+ struct Foo {
+Foo() {}
+ };
+}
+ )cpp";
+ EXPECT_UNAVAILABLE(R"cpp(
+namespace ns1 {
+ struct Bar {
+int x;
+int y;
+ };
+ void bar(Bar b) {}
+ void foo() {
+Bar b[[{}]];
+ }
+}
+ )cpp");
+ ExtraArgs.push_back("-std=c++20");
+ EXPECT_UNAVAILABLE(R"cpp(
+namespace ns1 {
+ struct Bar : Foo {
+int x;
+int y;
+ };
+ void bar(Bar b) {}
+ void foo() {
+Bar b[[{}]];
+ }
+}
+ )cpp");
+ EXPECT_AVAILABLE(R"cpp(
+namespace ns1 {
+ struct Bar {
+int x;
+int y;
+ };
+ void bar(Bar b) {}
+ void foo() {
+Bar b[[{}]];
+bar([[{}]]);
+ }
+}
+ )cpp");
+}
+
+TEST_F(FillStructDesignatedInitTest, Apply1) {
+ ExtraArgs.push_back("-std=c++20");
+ EXPECT_EQ(apply(R"cpp(
+namespace ns1 {
+ struct D1 {
+int x;
+int* y;
+ };
+ void foo() {
+D1 b[[{}]];
+ }
+}
+ )cpp"),
+R"cpp(
+namespace ns1 {
+ struct D1 {
+int x;
+int* y;
+ };
+ void foo() {
+D1 b{.x = int{}, .y = {}};
+ }
+}
+ )cpp");
+}
+
+TEST_F(FillStructDesignatedInitTest, Apply2) {
+ ExtraArgs.push_back("-std=c++20");
+ Header = R"cpp(
+namespace ns {
+ struct B1 {
+int b1;
+ };
+}
+namespace ns1 {
+ struct D1 {
+int* x;
+ns::B1 b1;
+ };
+}
+ )cpp";
+ EXPECT_EQ(apply(R"cpp(
+namespace ns1 {
+ void foo() {
+D1 b[[{}]];
+ }
+}
+ )cpp"),
+R"cpp(
+namespace ns1 {
+ void foo() {
+D1 b{.x = {}, .b1 = ns::B1{}};
+ }
+}
+ )cpp");
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -118,6 +118,7 @@
tweaks/ExpandMacroTests.cpp
tweaks/ExtractFunctionTests.cpp
tweaks/ExtractVariableTests.cpp
+ tweaks/FillStructDesignatedInitTests.cpp
tweaks/MemberwiseConstructorTests.cpp
tweaks/ObjCLocalizeStringLiteralTests.cpp
tweaks/ObjCMemberwiseInitializerTests.cpp
Index: clang-tools-extra/clangd/refactor/tweaks/FillStructDesignatedInit.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/FillStructDesignatedInit.cpp
@@ -0,0 +1,128 @@
+//===--- FillStructDesignatedInit.cpp ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "AST.h"
+#include "Selection.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// A tweak that fills an aggregate witho
[PATCH] D140275: [clangd] Tweak to add doxygen comment to the function declaration
tupos created this revision.
tupos added reviewers: sammccall, njames93.
tupos added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
tupos requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D140275
Files:
clang-tools-extra/clangd/refactor/tweaks/AddDoxygenComment.cpp
clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/tweaks/AddDoxygenCommentTests.cpp
Index: clang-tools-extra/clangd/unittests/tweaks/AddDoxygenCommentTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/tweaks/AddDoxygenCommentTests.cpp
@@ -0,0 +1,98 @@
+//===-- AddDoxygenCommentTests.cpp *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TweakTesting.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TWEAK_TEST(AddDoxygenComment);
+
+TEST_F(AddDoxygenCommentTest, AvailableUnavailable) {
+ EXPECT_AVAILABLE(R"cpp(
+void bar(char b) {^ }
+[[int foo(int x) { }]]
+namespace ns1 {
+ void bar(char b) {^ }
+ [[int foo(int x) { }]]
+}
+ )cpp");
+ EXPECT_UNAVAILABLE(R"cpp(
+namespace ns1 {
+ /*!
+ */
+ void bar(char b) {^ }
+ // comment
+ [[int foo(int x) { }]]
+ /// comment
+ [[int meaw(int x) { }]]
+}
+ )cpp");
+}
+
+TEST_F(AddDoxygenCommentTest, Apply1) {
+ EXPECT_EQ(apply(R"cpp(
+namespace ns1 {
+ int foo(int y, char* s) {
+^int x;
+return x;
+ }
+}
+ )cpp"),
+R"cpp(
+namespace ns1 {
+
+/*!
+ * @brief
+ *
+* @param y
+* @param s
+* @return
+*/
+int foo(int y, char* s) {
+int x;
+return x;
+ }
+}
+ )cpp");
+}
+
+TEST_F(AddDoxygenCommentTest, Apply2) {
+ EXPECT_EQ(apply(R"cpp(
+void foo();
+void ^bar();
+namespace ns1 {
+ int foo(int y, char* s) {
+int x;
+return x;
+ }
+}
+)cpp"),
+R"cpp(
+void foo();
+
+/*!
+ * @brief
+ *
+*/
+void bar();
+namespace ns1 {
+ int foo(int y, char* s) {
+int x;
+return x;
+ }
+}
+)cpp");
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -107,6 +107,7 @@
support/TestTracer.cpp
support/TraceTests.cpp
+ tweaks/AddDoxygenCommentTests.cpp
tweaks/AddUsingTests.cpp
tweaks/AnnotateHighlightingsTests.cpp
tweaks/DefineInlineTests.cpp
Index: clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
===
--- clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -12,6 +12,7 @@
# $ to a list of sources, see
# clangd/tool/CMakeLists.txt for an example.
add_clang_library(clangDaemonTweaks OBJECT
+ AddDoxygenComment.cpp
AddUsing.cpp
AnnotateHighlightings.cpp
DumpAST.cpp
Index: clang-tools-extra/clangd/refactor/tweaks/AddDoxygenComment.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/AddDoxygenComment.cpp
@@ -0,0 +1,147 @@
+//===--- AddDoxydgenComment.cpp -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "AST.h"
+#include "Selection.h"
+#include "refactor/InsertionPoint.h"
+#include "refactor/Tweak.h"
+#include "support/Logger.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// A tweak that adds a doxygen comment to the function declaration
+//
+// Given:
+// int func(int x, char const* s, Bar bar) {}
+// the tweak add the doxygen comment to the function in the form
+// /*!
+//* @brief
+//* @param x
+//* @param s
+//* @param bar
+//* @return
+//*/
+// int func(int x, char
[PATCH] D140275: [clangd] Tweak to add doxygen comment to the function declaration
tupos added a comment. I do not know but maybe it makes sence to make configurable the style of the comment, e.g instead of `@brief` use `\brief` and the same for `/*!` to `/**` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140275/new/ https://reviews.llvm.org/D140275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D140275: [clangd] Tweak to add doxygen comment to the function declaration
tupos added a comment. Also I tried to use it now, and I'm not sure where it is a good idea to allow this tweak from within a function body. Because it is quite annoying that in every function you get this code action. On the other hand maybe it is ok to allow it from within the body, but the tweak then should be smarter and should check whether on the function declaration the doxygen comment is already present. Also if the tweak is smarter it is fine to get this code action, as it will motivate me to get rid of it and provide a proper documentation ;-). What do you think? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140275/new/ https://reviews.llvm.org/D140275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D140275: [clangd] Tweak to add doxygen comment to the function declaration
tupos added a comment. Hi, sorry for the long delay, but actually in this particular moment I was working on addressing the comments. Therefore, I will push the updated patch soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140275/new/ https://reviews.llvm.org/D140275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D140275: [clangd] Tweak to add doxygen comment to the function declaration
tupos updated this revision to Diff 506220.
tupos marked 11 inline comments as done.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140275/new/
https://reviews.llvm.org/D140275
Files:
clang-tools-extra/clangd/refactor/tweaks/AddDoxygenComment.cpp
clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/tweaks/AddDoxygenCommentTests.cpp
Index: clang-tools-extra/clangd/unittests/tweaks/AddDoxygenCommentTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/tweaks/AddDoxygenCommentTests.cpp
@@ -0,0 +1,189 @@
+//===-- AddDoxygenCommentTests.cpp *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TweakTesting.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TWEAK_TEST(AddDoxygenComment);
+
+TEST_F(AddDoxygenCommentTest, AvailableUnavailable) {
+ EXPECT_AVAILABLE(R"cpp(
+void bar^(char b) { }
+[[int foo(int x)]] { }
+namespace ns1 {
+ [[int foo(int x)]] { }
+}
+ )cpp");
+ EXPECT_UNAVAILABLE(R"cpp(
+namespace ns1 {
+ /*!
+ */
+ void bar(char b) {^ }
+ // comment
+ [[int foo(int x) { }]]
+ /// comment
+ [[int meaw(int x) { }]]
+ void baz(char b) {^ }
+}
+ )cpp");
+}
+
+TEST_F(AddDoxygenCommentTest, AvailableUnavailableOnDefinition) {
+ EXPECT_AVAILABLE(R"cpp(
+void bar^(char b);
+void bar(char b) { (void)b; }
+ )cpp");
+ EXPECT_UNAVAILABLE(R"cpp(
+void bar(char b);
+void bar^(char b) { (void)b; }
+ )cpp");
+}
+
+TEST_F(AddDoxygenCommentTest, AvailableUnavailableOnMemberFunc) {
+ EXPECT_AVAILABLE(R"cpp(
+class Foo {
+ void foo^(){}
+ void bar^();
+};
+ )cpp");
+ EXPECT_UNAVAILABLE(R"cpp(
+class Foo {
+ void bar();
+};
+void Foo::bar^(){}
+ )cpp");
+}
+
+TEST_F(AddDoxygenCommentTest, AvailableUnavailableOnTemplate) {
+ EXPECT_AVAILABLE(R"cpp(
+template
+class Foo {
+ void foo^(){}
+ void bar^();
+};
+ )cpp");
+ EXPECT_UNAVAILABLE(R"cpp(
+template
+class Foo {
+ void bar();
+};
+template
+void Foo::bar^(){}
+ )cpp");
+}
+
+TEST_F(AddDoxygenCommentTest, AvailableUnavailableOnDeductionGuide) {
+ EXPECT_UNAVAILABLE(R"cpp(
+template struct A { A(); A(T); };
+A^() -> A;
+ )cpp");
+}
+
+TEST_F(AddDoxygenCommentTest, AvailableUnavailableObjC) {
+ FileName = "TestTU.m";
+ EXPECT_AVAILABLE(R"objc(
+@interface Foo
++ (id)^fooWithValue:(int)value fooey:(unsigned int)fooey;
+@end
+ )objc");
+}
+
+TEST_F(AddDoxygenCommentTest, ApplyTemplate) {
+ EXPECT_EQ(apply(R"cpp(
+template
+void foo^(T x) {})cpp"),
+R"cpp(
+/// TODO Add description
+///
+/// @param x
+///
+template
+void foo(T x) {})cpp");
+}
+
+TEST_F(AddDoxygenCommentTest, ApplyObjCIdReturn) {
+ FileName = "TestTU.m";
+ EXPECT_EQ(apply(R"objc(
+@interface Foo
++ (id)^fooWithValue:(int)value fooey:(unsigned int)fooey;
+@end)objc"),
+R"objc(
+@interface Foo
+/// TODO Add description
+///
+/// @param value
+/// @param fooey
+///
++ (id)fooWithValue:(int)value fooey:(unsigned int)fooey;
+@end)objc");
+}
+TEST_F(AddDoxygenCommentTest, ApplyObjCNonIdReturn) {
+ FileName = "TestTU.m";
+ EXPECT_EQ(apply(R"objc(
+@interface Foo
++ (int)^fooWithValue:(int)value fooey:(unsigned int)fooey;
+@end)objc"),
+R"objc(
+@interface Foo
+/// TODO Add description
+///
+/// @param value
+/// @param fooey
+/// @return
+///
++ (int)fooWithValue:(int)value fooey:(unsigned int)fooey;
+@end)objc");
+}
+
+TEST_F(AddDoxygenCommentTest, ApplyInsideNS) {
+ EXPECT_EQ(apply(R"cpp(
+namespace ns1 {
+ int foo^(int y, char* s) {
+int x;
+return x;
+ }
+}
+ )cpp"),
+R"cpp(
+namespace ns1 {
+ /// TODO Add description
+///
+/// @param y
+/// @param s
+/// @return
+///
+int foo(int y, char* s) {
+int x;
+return x;
+ }
+}
+ )cpp");
+}
+
+TEST_F(AddDoxygenCommentTest, ApplyInsideTU) {
+ EXPECT_EQ(apply(R"cpp(
+void foo();
+void ^bar();
+)cpp"),
+R"cpp(
+void foo();
+/// TODO Add description
+///
+///
+void bar();
+)cpp");
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -113,6 +113,7 @@
support/ThreadingTests.cpp
support/TraceTests.cpp
+ tweaks/AddDoxygenCommentTests.cpp
tweaks/AddUsingTests.cpp
[PATCH] D140275: [clangd] Tweak to add doxygen comment to the function declaration
tupos added inline comments.
Comment at: clang-tools-extra/clangd/refactor/tweaks/AddDoxygenComment.cpp:24
+namespace clang {
+namespace clangd {
+namespace {
tschuett wrote:
> You can merge this into `namespace clang::clangd`.
I prefer not to do it, because all other files with tweaks have the same style.
However, if it is still necessary please let me know and I will change it.
Comment at: clang-tools-extra/clangd/refactor/tweaks/AddDoxygenComment.cpp:37
+//* @param bar
+//* @return
+//*/
dgoldman wrote:
> sammccall wrote:
> > I'm a bit concerned about people generating these `@param bar` and
> > `@return` and leaving them there without filling them in - I've seen plenty
> > of code like that and it's substantially worse than no comments at all.
> >
> > I'm not sure we can do much though: could generate `@return TODO` or so to
> > make it more visually obvious - WDYT?
> VS Code itself has support for snippets -
> https://code.visualstudio.com/api/references/vscode-api#TextEditor -
> insertSnippet - but the LSP spec doesn't yet. Once it has it I think it would
> makes sense to use them here, but until then, TODO seems like the best we can
> do?
I do not think that adding here TODO makes much sense, because in this case I
can just type the whole comment automatically. Think about I added a template
but then I need to remove part of this template because it was a placeholder.
I personally think that people who do not want to fill the template properly
should blame themselves for not writing a proper comment and it is not a
responsibility of the clangd to motivate them to do it.
What I'm not sure about whether we need to add to the template @pre and @post
because this is more important as filling the parameters names.
Ideally it would be good if the template can be configurable through the
.clangd config. What do you think?
Comment at: clang-tools-extra/clangd/refactor/tweaks/AddDoxygenComment.cpp:58
+};
+
+REGISTER_TWEAK(AddDoxygenComment)
sammccall wrote:
> tschuett wrote:
> > The LLVM coding style kindly asks you stop the anonymous namespace here.
> > Furthermore, I believe that the `REGISTER_TWEAK` does not work in an
> > anonymous namespace.
> The coding style is sorely outdated on this point (indentation?!), and the
> code in this subproject doesn't follow it - happy to take this up on
> discourse if needed but I don't think we should create a mess of mixed local
> style here.
See the comment above.
Comment at: clang-tools-extra/clangd/refactor/tweaks/AddDoxygenComment.cpp:74
+bool AddDoxygenComment::prepare(const Selection &Inputs) {
+ if (!Inputs.AST->getLangOpts().CPlusPlus) {
+return false;
dgoldman wrote:
> sammccall wrote:
> > why? doxygen supports C AFAIK
> Would also be nice to support ObjC too, we'll just need to add support for
> ObjCMethodDecl as well to expand support for ObjC methods.
> (https://clang.llvm.org/doxygen/classclang_1_1ObjCMethodDecl.html vs
> https://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html) But even if
> that's not done in this diff, still seems fine to enable it generally?
What is the correct language option for the C language? I found only the
enumeration of all C standards?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140275/new/
https://reviews.llvm.org/D140275
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D140275: [clangd] Tweak to add doxygen comment to the function declaration
tupos added a comment. Could you please also advice me what else need to be done for the ObjC, since there were many years since I wrote ObjC last time I'm not sure what else need to be done there. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140275/new/ https://reviews.llvm.org/D140275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D140275: [clangd] Tweak to add doxygen comment to the function declaration
tupos added a comment. Hi, could you please provide a code review again? Thanks. With best regards, Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140275/new/ https://reviews.llvm.org/D140275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
