Csdrhrt

When generating a string representation of expression trees, I would like to render calls to String.Format with a constant string as the first parameter, as interpolated strings (link). In order to do so, I have to parse the compiler-generated composite format string.

I've written the following function for this purpose, based on the .NET Core parsing function. I am looking primarily for review in:

• correctness, even in edge cases


• clarity / readability

The function returns an array of tuples. Each tuple contains the following elements:

1. literal string until the next placeholder


2. index of placeholder


3. alignment


4. item format

If there is literal text after the last placeholder, it will be added as the last tuple of the array (other elements of the tuple will be null).

The function defines 3 local functions:

• 
  advanceChar -- advances the current position (pos) by one character, and stores the current character (ch)


• 
  skipWhitespace -- advances the current position as long as the current character is a space


• 
  getNumber -- gets a multi-digit number starting from the current position; ignores leading/trailing whitespace

public static (string literal, int? index, int? alignment, string itemFormat) ParseFormatString(string format) {

    const int indexLimit = 1000000;

    const int alignmentLimit = 100000;



    int pos = -1;

    char ch = 'x0';

    int lastPos = format.Length - 1;



    var parts = new List<(string literal, int? index, int? alignment, string itemFormat)>();



    while (true) {



        // Parse literal until argument placeholder

        string literal = "";

        while (pos < lastPos) {

            advanceChar();



            if (ch == '}') {

                advanceChar();

                if (ch == '}') {

                    literal += '}';

                } else {

                    throw new Exception("Mismatched end brace");

                }

            } else if (ch == '{') {

                advanceChar();

                if (ch == '{') {

                    literal += '{';

                } else {

                    break;

                }

            } else {

                literal += ch;

            }

        }



        if (pos == lastPos) {

            if (literal != "") {

                parts.Add((literal, (int?)null, (int?)null, (string)null));

            }

            break;

        }



        // Parse index section; required

        int index = getNumber(indexLimit);



        // Parse alignment; optional

        int? alignment = null;

        if (ch == ',') {

            advanceChar();

            alignment = getNumber(alignmentLimit, true);

        }



        // Parse item format; optional

        string itemFormat = null;

        if (ch == ':') {

            advanceChar();

            if (ch == '{') {

                advanceChar();

                if (ch == '{') {

                    itemFormat += '{';

                } else {

                    throw new Exception("Nested placeholders not allowed");

                }

            } else if (ch == '}') {

                advanceChar();

                if (ch=='}') {

                    itemFormat += '}';

                } else {

                    break;

                }

            } else {

                itemFormat += ch;

            }

        }



        parts.Add((literal, index, alignment, itemFormat));

    }



    return parts.ToArray();



    void advanceChar(bool ignoreEnd = false) {

        pos += 1;

        if (pos <= lastPos) {

            ch = format[pos];

        } else if (ignoreEnd) {

            ch = 'x0';

        } else {

            throw new Exception("Unexpected end of text");

        }

    }



    void skipWhitespace() {

        while (ch == ' ') {

            advanceChar(true);

        }

    }



    int getNumber(int limit, bool allowNegative = false) {

        skipWhitespace();



        bool isNegative = false;

        if (allowNegative && ch == '-') {

            isNegative = true;

            advanceChar();

        }

        if (ch < '0' || ch > '9') { throw new Exception("Expected digit"); }

        int ret = 0;

        do {

            ret = ret * 10 + ch - '0';

            advanceChar();

        } while (ch >= '0' && ch <= '9' && ret < limit);



        skipWhitespace();



        return ret * (isNegative ? -1 : 1);

    }

}

Problems:

Item format parsing is broken:

• 
  "{0:X2}" fails with an 'Unexpected end of text' exception, while "{0:X2}a" fails with a 'Mismatched end brace' exception. Both are valid formats.


• 
  "{0:}" also fails with an 'Unexpected end of text' exception, but "{0:}a" returns an empty array instead. Both are valid formats.


• 
  "{0:{{" and "{0:}}" are parsed successfully. Both should be rejected as invalid.

Improvements:

• The index and alignment limits seem fairly arbitrary. If they're based on an actual limit it would be a good idea to document that. Also, exceeding those limits results in a misleading 'Mismatched end brace' error.


• I'd recommend using a more specific exception exception type. The existing FormatException seems appropriate here.


• For repeated string concatenation, a StringBuilder is (significantly) more efficient.


• The exceptions don't provide much detail. It would be useful to know at what index the problem was detected, or what the parser was expecting when it hit the end of the input.


• The main while loop body is fairly drawn out. If you're using local functions anyway, why not split things up further into a ParseLiteralPart and ParseFormatPart function?