[clang-format] Do not treat C# attribute targets as labels

[jbcoe]

Summary

Merge '[', 'target' , ':' into a single token for C# attributes to prevent the target from being seen as a label.

[krasimir]

This seems OK, but it feels a bit messy that the text [target: becomes the opening square bracket. I think this might be just good enough for these attributes, so I'm ok with this for now. But a more principled solution (for later, if we hit any dead ends with this) would be to introduce a new AnnotateToken type, like TT_CSharpAttributeTarget and make sure these are annotated properly.

clang/lib/Format/FormatTokenLexer.cpp

286

Here Target is unconstrained. Can it be anything? Maybe we should check if it's an tok::identifier?

Even better, seems we can even enumerate the possibilities:
https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/attributes/#attribute-targets

[jbcoe]

Check target type against list of C# attribute targets in https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/attributes/#attribute-targets

[jbcoe]

This definitely feels good enough rather than ideal. We should investigate adding a new token type if this 'fix' proves to have shortcomings.

[krasimir]

clang/lib/Format/FormatTokenLexer.cpp

299

Consider using a const llvm::StringSet<> like in:
https://github.com/llvm/llvm-project/blob/70c98671fa7b395a52829b91782393f4c2613562/clang/lib/Format/BreakableToken.cpp#L543

[jbcoe]

Use StringSet to list valid C# attribute targets.

[MyDeveloperDay]

LGTM, thanks for the patch (I've seen this in the wild where the AssemblyInfo.cs gets a bit messed up.

clang/lib/Format/FormatTokenLexer.cpp

287

+1 for just supporting what it can be.

clang/lib/Format/FormatTokenLexer.h

 #define LLVM_CLANG_LIB_FORMAT_FORMATTOKENLEXER_H
 
 #include "Encoding.h"
 #include "FormatToken.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Regex.h"
 
 #include <stack>
 
 namespace clang {
 namespace format {
 
 enum LexerState {
private:
   bool tryMergeLessLess();
   bool tryMergeNSStringLiteral();
   bool tryMergeJSPrivateIdentifier();
   bool tryMergeCSharpStringLiteral();
   bool tryMergeCSharpKeywordVariables();
   bool tryMergeCSharpNullConditionals();
   bool tryMergeCSharpDoubleQuestion();
   bool tryTransformCSharpForEach();
+  bool tryMergeCSharpAttributeAndTarget();
 
   bool tryMergeTokens(ArrayRef<tok::TokenKind> Kinds, TokenType NewType);
 
   // Returns \c true if \p Tok can only be followed by an operand in JavaScript.
   bool precedesOperand(FormatToken *Tok);
 
   bool canPrecedeRegexLiteral(FormatToken *Prev);
 
private:
 
   llvm::SmallMapVector<IdentifierInfo *, TokenType, 8> Macros;
 
   bool FormattingDisabled;
 
   llvm::Regex MacroBlockBeginRegex;
   llvm::Regex MacroBlockEndRegex;
 
+  // Targets that may appear inside a C# attribute.
+  static const llvm::StringSet<> CSharpAttributeTargets;
+
   void readRawToken(FormatToken &Tok);
 
   void resetLexer(unsigned Offset);
 };
 
 } // namespace format
 } // namespace clang
 
 #endif

clang/lib/Format/FormatTokenLexer.cpp

void FormatTokenLexer::tryMergePreviousTokens() {
   if (tryMerge_TMacro())
     return;
   if (tryMergeConflictMarkers())
     return;
   if (tryMergeLessLess())
     return;
 
   if (Style.isCSharp()) {
+    if (tryMergeCSharpAttributeAndTarget())
+      return;
     if (tryMergeCSharpKeywordVariables())
       return;
     if (tryMergeCSharpStringLiteral())
       return;
     if (tryMergeCSharpDoubleQuestion())
       return;
     if (tryMergeCSharpNullConditionals())
       return;
bool FormatTokenLexer::tryMergeCSharpStringLiteral() {
   At->TokenText = StringRef(At->TokenText.begin(),
                             String->TokenText.end() - At->TokenText.begin());
   At->ColumnWidth += String->ColumnWidth;
   At->Type = TT_CSharpStringLiteral;
   Tokens.erase(Tokens.end() - 1);
   return true;
 }
 
+// Valid C# attribute targets:
+// https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/attributes/#attribute-targets
+const llvm::StringSet<> FormatTokenLexer::CSharpAttributeTargets = {
+    "assembly", "module",   "field",  "event", "method",
+    "param",    "property", "return", "type",
+};
+
+bool FormatTokenLexer::tryMergeCSharpAttributeAndTarget() {
+  // Treat '[assembly:' and '[field:' as tokens in their own right.
+  if (Tokens.size() < 3)
+    return false;
+
+  auto &SquareBracket = *(Tokens.end() - 3);
+  auto &Target = *(Tokens.end() - 2);
+  auto &Colon = *(Tokens.end() - 1);
+
+  if (!SquareBracket->Tok.is(tok::l_square))
+    return false;
+
+  if (CSharpAttributeTargets.find(Target->TokenText) ==
+      CSharpAttributeTargets.end())
+    return false;
+
+  if (!Colon->Tok.is(tok::colon))
+    return false;
+
+  SquareBracket->TokenText =
+      StringRef(SquareBracket->TokenText.begin(),
+                Colon->TokenText.end() - SquareBracket->TokenText.begin());
+  SquareBracket->ColumnWidth += (Target->ColumnWidth + Colon->ColumnWidth);
+  Tokens.erase(Tokens.end() - 2);
+  Tokens.erase(Tokens.end() - 1);
+  return true;
+}
+
 bool FormatTokenLexer::tryMergeCSharpDoubleQuestion() {
   if (Tokens.size() < 2)
     return false;
   auto &FirstQuestion = *(Tokens.end() - 2);
   auto &SecondQuestion = *(Tokens.end() - 1);
   if (!FirstQuestion->is(tok::question) || !SecondQuestion->is(tok::question))
     return false;
   FirstQuestion->Tok.setKind(tok::question);

clang/unittests/Format/FormatTestCSharp.cpp

verifyFormat("[TestMethod(\"start\", HelpText = \"Starts the server "
                "    set;\n"
                "    get;\n"
                "}");
 
   verifyFormat(
       "[DllImport(\"Hello\", EntryPoint = \"hello_world\")]\n"
       "// The const char* returned by hello_world must not be deleted.\n"
       "private static extern IntPtr HelloFromCpp();)");
+
+  //  Unwrappable lines go on a line of their own.
+  // 'target:' is not treated as a label.
+  // Modify Style to enforce a column limit.
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.ColumnLimit = 10;
+  verifyFormat(R"([assembly:InternalsVisibleTo(
+    "SomeAssembly, PublicKey=SomePublicKeyThatExceedsTheColumnLimit")])",
+               Style);
 }
 
 TEST_F(FormatTestCSharp, CSharpUsing) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
   Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
   verifyFormat("public void foo () {\n"
                "  using (StreamWriter sw = new StreamWriter (filenameA)) {}\n"
                "  using () {}\n"