[cland] Dont emit missing newline warnings when building preamble

[kadircet]

Summary

When building preamble, clangd truncates file contents. This yielded
errnous warnings in some cases.

This patch fixes the issue by turning off no-newline-at-eof warnings whenever
the file has more contents than the preamble.

Fixes https://github.com/clangd/clangd/issues/744.

[hokein]

Thanks! And sorry for the delay, I somehow missed it in my inbox.

clang-tools-extra/clangd/Preamble.cpp

350

it seems that there is another similar warning warn_cxx98_compat_no_newline_eof, I think we probably should cover it.

[kadircet]

• Handle cxx98 and extension versions of the warning too.

[sammccall]

A too-late drive-by - maybe we'd be better detecting these in clang never emitting these

[sammccall]

Hit send too soon. But IIRC PP knows whether it's building a preamble.

[kadircet]

Hit send too soon. But IIRC PP knows whether it's building a preamble.

That's a good point. But PP doesn't know if there's trailing content or not (in clangd's case).
So we'll end up not showing the diag even when we should (i.e. whole file is preamble without a new line at eof).
It doesn't sound too bad, but it'll effect all tools building preambles (including clang) and not just clangd so I am a little hesitant.

clang-tools-extra/clangd/Preamble.cpp

 #include "Compiler.h"
 #include "Headers.h"
 #include "SourceCode.h"
 #include "support/Logger.h"
 #include "support/ThreadsafeFS.h"
 #include "support/Trace.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticLex.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/PPCallbacks.h"
buildPreamble(PathRef FileName, CompilerInvocation CI,
   PreambleDiagnostics.setDiagCallback(
       [&ASTListeners](const clang::Diagnostic &D, clangd::Diag &Diag) {
         llvm::for_each(ASTListeners,
                        [&](const auto &L) { L->sawDiagnostic(D, Diag); });
       });
   llvm::IntrusiveRefCntPtr<DiagnosticsEngine> PreambleDiagsEngine =
       CompilerInstance::createDiagnostics(&CI.getDiagnosticOpts(),
                                           &PreambleDiagnostics, false);
+  PreambleDiagnostics.setLevelAdjuster(
+      [&](DiagnosticsEngine::Level DiagLevel, const clang::Diagnostic &Info) {
+        switch (Info.getID()) {
+        case diag::warn_no_newline_eof:
+        case diag::warn_cxx98_compat_no_newline_eof:
+        case diag::ext_no_newline_eof:
+          // If the preamble doesn't span the whole file, drop the no newline at
+          // eof warnings.
+          return Bounds.Size != ContentsBuffer->getBufferSize()
+                     ? DiagnosticsEngine::Level::Ignored
+                     : DiagLevel;
+        }
+        return DiagLevel;
+      });
 
   // Skip function bodies when building the preamble to speed up building
   // the preamble and make it smaller.
   assert(!CI.getFrontendOpts().SkipFunctionBodies);
   CI.getFrontendOpts().SkipFunctionBodies = true;
   // We don't want to write comment locations into PCH. They are racy and slow
   // to read back. We rely on dynamic index for the comments instead.
   CI.getPreprocessorOpts().WriteCommentListToPCH = false;

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

TEST(ParsedASTTest, ModuleSawDiag) {
   TestTU TU;
   TU.Code = Code.code().str();
   TU.FeatureModules = &FMS;
 
   auto AST = TU.build();
   EXPECT_THAT(*AST.getDiagnostics(),
               testing::Contains(Diag(Code.range(), KDiagMsg.str())));
 }
+
+TEST(Preamble, EndsOnNonEmptyLine) {
+  TestTU TU;
+  TU.ExtraArgs = {"-Wnewline-eof"};
+
+  {
+    TU.Code = "#define FOO\n  void bar();\n";
+    auto AST = TU.build();
+    EXPECT_THAT(*AST.getDiagnostics(), IsEmpty());
+  }
+  {
+    Annotations Code("#define FOO[[]]");
+    TU.Code = Code.code().str();
+    auto AST = TU.build();
+    EXPECT_THAT(
+        *AST.getDiagnostics(),
+        testing::Contains(Diag(Code.range(), "no newline at end of file")));
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang