Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Format] Fix detection of languages when reading from stdin #79051

Merged
merged 1 commit into from
Jan 23, 2024

Conversation

bhamiltoncx
Copy link
Contributor

The code cleanup in #74794 accidentally broke detection of languages by reading file content from stdin, e.g. via clang-format -dump-config - < /path/to/filename.

This PR adds unit and integration tests to reproduce the issue and adds a fix.

Fixes: #79023

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 22, 2024

@llvm/pr-subscribers-clang-format

Author: Ben Hamilton (Ben Gertzfield) (bhamiltoncx)

Changes

The code cleanup in #74794 accidentally broke detection of languages by reading file content from stdin, e.g. via clang-format -dump-config - &lt; /path/to/filename.

This PR adds unit and integration tests to reproduce the issue and adds a fix.

Fixes: #79023


Full diff: https://github.com/llvm/llvm-project/pull/79051.diff

3 Files Affected:

  • (added) clang/test/Format/dump-config-objc-stdin.m (+5)
  • (modified) clang/tools/clang-format/ClangFormat.cpp (+13-11)
  • (modified) clang/unittests/Format/FormatTestObjC.cpp (+8)
diff --git a/clang/test/Format/dump-config-objc-stdin.m b/clang/test/Format/dump-config-objc-stdin.m
new file mode 100644
index 000000000000000..b22ff7b3328caa5
--- /dev/null
+++ b/clang/test/Format/dump-config-objc-stdin.m
@@ -0,0 +1,5 @@
+// RUN: clang-format -dump-config - < %s | FileCheck %s
+
+// CHECK: Language: ObjC
+@interface Foo
+@end
diff --git a/clang/tools/clang-format/ClangFormat.cpp b/clang/tools/clang-format/ClangFormat.cpp
index 49ab7677a3ee9c2..9cd61ed903a551b 100644
--- a/clang/tools/clang-format/ClangFormat.cpp
+++ b/clang/tools/clang-format/ClangFormat.cpp
@@ -547,18 +547,20 @@ static void PrintVersion(raw_ostream &OS) {
 // Dump the configuration.
 static int dumpConfig(bool IsSTDIN) {
   std::unique_ptr<llvm::MemoryBuffer> Code;
-  // We can't read the code to detect the language if there's no file name.
-  if (!IsSTDIN) {
-    // Read in the code in case the filename alone isn't enough to detect the
-    // language.
-    ErrorOr<std::unique_ptr<MemoryBuffer>> CodeOrErr =
-        MemoryBuffer::getFileOrSTDIN(FileNames[0]);
-    if (std::error_code EC = CodeOrErr.getError()) {
-      llvm::errs() << EC.message() << "\n";
-      return 1;
-    }
-    Code = std::move(CodeOrErr.get());
+
+  // FileNames should have at least "-" in it even if no file was specified.
+  assert(!FileNames.empty());
+
+  // Read in the code in case the filename alone isn't enough to detect the
+  // language.
+  ErrorOr<std::unique_ptr<MemoryBuffer>> CodeOrErr =
+      MemoryBuffer::getFileOrSTDIN(FileNames[0]);
+  if (std::error_code EC = CodeOrErr.getError()) {
+    llvm::errs() << EC.message() << "\n";
+    return 1;
   }
+  Code = std::move(CodeOrErr.get());
+
   llvm::Expected<clang::format::FormatStyle> FormatStyle =
       clang::format::getStyle(Style, IsSTDIN ? AssumeFileName : FileNames[0],
                               FallbackStyle, Code ? Code->getBuffer() : "");
diff --git a/clang/unittests/Format/FormatTestObjC.cpp b/clang/unittests/Format/FormatTestObjC.cpp
index cd4f9d934127bfd..d2c3459e0f846d7 100644
--- a/clang/unittests/Format/FormatTestObjC.cpp
+++ b/clang/unittests/Format/FormatTestObjC.cpp
@@ -31,6 +31,14 @@ class FormatTestObjC : public FormatTestBase {
   _verifyIncompleteFormat(__FILE__, __LINE__, __VA_ARGS__)
 #define verifyFormat(...) _verifyFormat(__FILE__, __LINE__, __VA_ARGS__)
 
+TEST(FormatTestObjCStyle, DetectsObjCInStdin) {
+  auto Style = getStyle("LLVM", "<stdin>", "none",
+                        "@interface\n"
+                        "- (id)init;");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+}
+
 TEST(FormatTestObjCStyle, DetectsObjCInHeaders) {
   auto Style = getStyle("LLVM", "a.h", "none",
                         "@interface\n"

@bhamiltoncx bhamiltoncx merged commit d813af7 into llvm:main Jan 23, 2024
7 checks passed
@bhamiltoncx bhamiltoncx deleted the issue-79023 branch January 23, 2024 20:32
@owenca
Copy link
Contributor

owenca commented Feb 4, 2024

@bhamiltoncx Thanks for the fix! Nevertheless, it seems that this triggered a new regression. See #80621.

@bhamiltoncx
Copy link
Contributor Author

bhamiltoncx commented Feb 5, 2024 via email

@owenca
Copy link
Contributor

owenca commented Feb 6, 2024

Probably. See #80628 for a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Format] Regression in clang-format - -dump_config < path/to/objc_file.m (now outputs Language: Cpp)
4 participants