Repository: dotnet/roslyn
File: src/Compilers/CSharp/Portable/Parser/SyntaxParser.cs (C#)

Code before:

        protected TNode AddError<TNode>(TNode node, ErrorCode code) where TNode : GreenNode
        {
            return AddError(node, code, Array.Empty<object>());
        }

        protected TNode AddErrorAsWarning<TNode>(TNode node, ErrorCode code, params object[] args) where TNode : GreenNode
        {
            Debug.Assert(!node.IsMissing);
            return AddError(node, ErrorCode.WRN_ErrorOverride, MakeError(node, code, args), (int)code);
        }

        protected TNode AddError<TNode>(TNode nodeOrToken, ErrorCode code, params object[] args) where TNode : GreenNode
        {
            if (!nodeOrToken.IsMissing)
            {
                // We have a normal node or token that has actual SyntaxToken.Text within it (or the EOF token). Place
                // the diagnostic at the start (not full start) of that real node/token, with a width that encompasses
                // the entire normal width of the node or token.
                Debug.Assert(nodeOrToken.Width > 0 || nodeOrToken.RawKind is (int)SyntaxKind.EndOfFileToken);
                return WithAdditionalDiagnostics(nodeOrToken, MakeError(nodeOrToken, code, args));
            }
            else
            {
                var (offset, width) = this.GetDiagnosticSpanForMissingNodeOrToken(nodeOrToken);
                return WithAdditionalDiagnostics(nodeOrToken, MakeError(offset, width, code, args));
            }
        }

        /// <summary>
        /// Given a "missing" node or token (one where <see cref="GreenNode.IsMissing"/> must be true, determines the
        /// ideal location to place the diagnostic for it.  The intuition here is that we want to place the diagnostic
        /// on the token that "follows" this 'missing' entity if they're on the same line.  Or, place it at the end of
        /// the 'preceding' token if the following token is on the next line.
        /// </summary>
        protected (int offset, int width) GetDiagnosticSpanForMissingNodeOrToken(GreenNode missingNodeOrToken)
        {
            Debug.Assert(missingNodeOrToken.IsMissing);

            // Note: missingNodeOrToken.IsMissing means this is either a MissingToken itself, or a node comprised
            // (transitively) only from MissingTokens.  Missing tokens are guaranteed to have no text.  But they are
            // allowed to have trivia.  This is a common pattern the parser will follow when it encounters unexpected
            // tokens.  It will make a missing token of the expected kind for the current location, then attach the
            // unexpected tokens as missed tokens to it.

            // At this point, we have a node or token without real text in it.  The intuition we have here is that we
            // want to place the diagnostic on the token that "follows" this 'missing' entity.  There is a subtlety
            // here.  If the node or token contains skipped tokens, then we consider that skipped token the "following"
            // token, and we will want to place the diagnostic on it.  Otherwise, we want to place it on the true 'next
            // token' the parser is currently pointing at.


Review comment (suggestion):
core change here.  instead of hte logic for this computation being here (and thus, only performed if AddError itself was calleD).  It's now moved into GetDiagnosticSpanForMissingNodeOrToken, so that anyone that is computing diagnostics for missing thigns gets the same logic. 

Code after:

        protected TNode AddError<TNode>(TNode node, ErrorCode code) where TNode : GreenNode
        {
            return AddError(node, code, Array.Empty<object>());
        }

        protected TNode AddErrorAsWarning<TNode>(TNode node, ErrorCode code, params object[] args) where TNode : GreenNode
        {
            Debug.Assert(!node.IsMissing);
            return AddError(node, ErrorCode.WRN_ErrorOverride, MakeError(node, code, args), (int)code);
        }

        protected TNode AddError<TNode>(TNode nodeOrToken, ErrorCode code, params object[] args) where TNode : GreenNode
        {
            if (!nodeOrToken.IsMissing)
            {
                // We have a normal node or token that has actual SyntaxToken.Text within it (or the EOF token). Place
                // the diagnostic at the start (not full start) of that real node/token, with a width that encompasses
                // the entire normal width of the node or token.
                Debug.Assert(nodeOrToken.Width > 0 || nodeOrToken.RawKind is (int)SyntaxKind.EndOfFileToken);
                return WithAdditionalDiagnostics(nodeOrToken, MakeError(nodeOrToken, code, args));
            }
            else
            {
                var (offset, width) = this.GetDiagnosticSpanForMissingNodeOrToken(nodeOrToken);
                return WithAdditionalDiagnostics(nodeOrToken, MakeError(offset, width, code, args));
            }
        }

        /// <summary>
        /// Given a "missing" node or token (one where <see cref="GreenNode.IsMissing"/> must be true), determines the
        /// ideal location to place the diagnostic for it.  The intuition here is that we want to place the diagnostic
        /// on the token that "follows" this 'missing' entity if they're on the same line.  Or, place it at the end of
        /// the 'preceding' token if the following token is on the next line.
        /// </summary>
        protected (int offset, int width) GetDiagnosticSpanForMissingNodeOrToken(GreenNode missingNodeOrToken)
        {
            Debug.Assert(missingNodeOrToken.IsMissing);

            // Note: missingNodeOrToken.IsMissing means this is either a MissingToken itself, or a node comprised
            // (transitively) only from MissingTokens.  Missing tokens are guaranteed to have no text.  But they are
            // allowed to have trivia.  This is a common pattern the parser will follow when it encounters unexpected
            // tokens.  It will make a missing token of the expected kind for the current location, then attach the
            // unexpected tokens as missed tokens to it.

            // At this point, we have a node or token without real text in it.  The intuition we have here is that we
            // want to place the diagnostic on the token that "follows" this 'missing' entity.  There is a subtlety
            // here.  If the node or token contains skipped tokens, then we consider that skipped token the "following"
            // token, and we will want to place the diagnostic on it.  Otherwise, we want to place it on the true 'next
            // token' the parser is currently pointing at.