[yaml] Error on alias nodes that have properties attached

[njames93]

Summary

Alias nodes are explicitly forbidden from having properties.
Currently this is handled by just ignoring the properties which is not correct and will lead to confusion if a user tries to attach a property to an alias and not get the expected behaviour.

[njames93]

Emit fix hints for removing the tags.

llvm/lib/Support/YAMLParser.cpp

public:
 
   /// Parse the next token and return it without popping it.
   Token &peekNext();
 
   /// Parse the next token and pop it from the queue.
   Token getNext();
 
   void printError(SMLoc Loc, SourceMgr::DiagKind Kind, const Twine &Message,
-                  ArrayRef<SMRange> Ranges = None) {
-    SM.PrintMessage(Loc, Kind, Message, Ranges, /* FixIts= */ None, ShowColors);
+                  ArrayRef<SMRange> Ranges = None,
+                  ArrayRef<SMFixIt> Fixes = None) {
+    SM.PrintMessage(Loc, Kind, Message, Ranges, Fixes, ShowColors);
   }
 
-  void setError(const Twine &Message, StringRef::iterator Position) {
+  void setError(const Twine &Message, StringRef::iterator Position,
+                ArrayRef<SMRange> Ranges = None,
+                ArrayRef<SMFixIt> Fixes = None) {
     if (Position >= End)
       Position = End - 1;
 
     // propagate the error if possible
     if (EC)
       *EC = make_error_code(std::errc::invalid_argument);
 
     // Don't print out more errors after the first one we encounter. The rest
     // are just the result of the first, and have no meaning.
     if (!Failed)
-      printError(SMLoc::getFromPointer(Position), SourceMgr::DK_Error, Message);
+      printError(SMLoc::getFromPointer(Position), SourceMgr::DK_Error, Message,
+                 Ranges, Fixes);
     Failed = true;
   }
 
   /// Returns true if an error occurred while parsing.
   bool failed() {
     return Failed;
   }
 

 void Document::setError(const Twine &Message, Token &Location) const {
   stream.scanner->setError(Message, Location.Range.begin());
 }
 
 bool Document::failed() const {
   return stream.scanner->failed();
 }
 
+static SMRange getRangeFromStringRef(StringRef Str) {
+  return SMRange(SMLoc::getFromPointer(Str.begin()),
+                 SMLoc::getFromPointer(Str.end()));
+}
+
 Node *Document::parseBlockNode() {
   Token T = peekNext();
   // Handle properties.
   Token AnchorInfo;
   Token TagInfo;
 parse_property:
   switch (T.Kind) {
-  case Token::TK_Alias:
-    getNext();
-    return new (NodeAllocator) AliasNode(stream.CurrentDoc, T.Range.substr(1));
   case Token::TK_Anchor:
     if (AnchorInfo.Kind == Token::TK_Anchor) {
       setError("Already encountered an anchor for this node!", T);
       return nullptr;
     }
     AnchorInfo = getNext(); // Consume TK_Anchor.
     T = peekNext();
     goto parse_property;
   case Token::TK_Tag:
     if (TagInfo.Kind == Token::TK_Tag) {
       setError("Already encountered a tag for this node!", T);
       return nullptr;
     }
     TagInfo = getNext(); // Consume TK_Tag.
     T = peekNext();
     goto parse_property;
   default:
     break;
   }
 
   switch (T.Kind) {
+  case Token::TK_Alias:
+    getNext();
+    // YAML1.2 - 7.1 Alias Nodes
+    //
+    // Note that an alias node must not specify any properties or content, as
+    // these were already specified at the first occurrence of the node.
+    if (AnchorInfo.Kind == Token::TK_Anchor || TagInfo.Kind == Token::TK_Tag) {
+      // If there was already an error, the below error message won't be
+      // emitted.
+      bool EmitFixes = !stream.scanner->failed();
+      stream.scanner->setError("Alias node cannot have any properties",
+                               T.Range.begin(),
+                               {getRangeFromStringRef(T.Range)});
+      if (EmitFixes) {
+        bool HasAnchor = AnchorInfo.Kind == Token::TK_Anchor;
+        bool HasTag = TagInfo.Kind == Token::TK_Tag;
+        const char *Begin;
+        StringRef Warning;
+        SmallVector<SMFixIt, 2> Fixes;
+        if (HasAnchor) {
+          if (HasTag) {
+            Begin = std::min(AnchorInfo.Range.begin(), TagInfo.Range.begin());
+            Warning = "remove anchor and tag";
+          } else {
+            Begin = AnchorInfo.Range.begin();
+            Warning = "remove anchor";
+          }
+        } else {
+          Begin = TagInfo.Range.begin();
+          Warning = "remove tag";
+        }
+        if (HasAnchor)
+          Fixes.emplace_back(getRangeFromStringRef(AnchorInfo.Range), "");
+        if (HasTag)
+          Fixes.emplace_back(getRangeFromStringRef(TagInfo.Range), "");
+        stream.scanner->printError(SMLoc::getFromPointer(Begin),
+                                   SourceMgr::DK_Note, Warning, {}, Fixes);
+      }
+      return nullptr;
+    }
+    return new (NodeAllocator) AliasNode(stream.CurrentDoc, T.Range.substr(1));
   case Token::TK_BlockEntry:
     // We got an unindented BlockEntry sequence. This is not terminated with
     // a BlockEnd.
     // Don't eat the TK_BlockEntry, SequenceNode needs it.
     return new (NodeAllocator) SequenceNode( stream.CurrentDoc
                                            , AnchorInfo.Range.substr(1)
                                            , TagInfo.Range
                                            , SequenceNode::ST_Indentless);

llvm/unittests/Support/YAMLParserTest.cpp

TEST(YAMLParser, HandlesEndOfFileGracefully) {
   ExpectParseError("In object hitting EOF", "{\"\"");
 }
 
 TEST(YAMLParser, HandlesNullValuesInKeyValueNodesGracefully) {
   ExpectParseError("KeyValueNode with null key", "? \"\n:");
   ExpectParseError("KeyValueNode with null value", "test: '");
 }
 
+TEST(YAMLParser, FailsOnAliasProperties) {
+  ExpectParseError("Alias node cannot have any properties", "&A *B");
+  ExpectParseError("Alias node cannot have any properties", "!A *B");
+  ExpectParseError("Alias node cannot have any properties", "!A &A *B");
+}
+
 // Checks that the given string can be parsed into an identical string inside
 // of an array.
 static void ExpectCanParseString(StringRef String) {
   std::string StringInArray = (llvm::Twine("[\"") + String + "\"]").str();
   SourceMgr SM;
   yaml::Stream Stream(StringInArray, SM);
   yaml::SequenceNode *ParsedSequence
     = dyn_cast<yaml::SequenceNode>(Stream.begin()->getRoot());