From 0b3946f9f55327cc723f9c86b1a889951863adbd Mon Sep 17 00:00:00 2001 From: Daniel Zuncke Date: Fri, 20 Oct 2023 17:51:55 +0200 Subject: [PATCH] Fix issue 578 Error description: In formatColon() the colon that should match a question mark gets selected as the default case of the if-else ladder which is a guaranteed " : ". Amongst the previous one there is the associated array matcher which is selected since this is happening in an aa (it just checks for surrounding []). Solution: Harden associated array if-condition. Existing data does not contain enough information, store ternary conditional colon indices in an array in ASTInformation. Before an associated array is matched, confirm that it isn't part of ternary conditional. If it is a ternary colon, it will fall through the if-ladder like before. --- src/dfmt/ast_info.d | 9 ++++++++ src/dfmt/formatter.d | 3 ++- tests/allman/issue0578.d.ref | 43 ++++++++++++++++++++++++++++++++++++ tests/issue0578.d | 43 ++++++++++++++++++++++++++++++++++++ tests/knr/issue0578.d.ref | 43 ++++++++++++++++++++++++++++++++++++ tests/otbs/issue0578.d.ref | 42 +++++++++++++++++++++++++++++++++++ 6 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 tests/allman/issue0578.d.ref create mode 100644 tests/issue0578.d create mode 100644 tests/knr/issue0578.d.ref create mode 100644 tests/otbs/issue0578.d.ref diff --git a/src/dfmt/ast_info.d b/src/dfmt/ast_info.d index 56030f1..88dbdff 100644 --- a/src/dfmt/ast_info.d +++ b/src/dfmt/ast_info.d @@ -63,6 +63,7 @@ struct ASTInformation (structInfoSortedByEndLocation); sort(ufcsHintLocations); ufcsHintLocations = ufcsHintLocations.uniq().array(); + sort(ternaryColonLocations); } /// Locations of end braces for struct bodies @@ -135,6 +136,9 @@ struct ASTInformation /// Opening & closing braces of struct initializers StructInitializerInfo[] structInfoSortedByEndLocation; + + /// Locations ternary expression colons. + size_t[] ternaryColonLocations; } /// Collects information from the AST that is useful for the formatter @@ -438,6 +442,11 @@ final class FormatVisitor : ASTVisitor outStatement.accept(this); } + override void visit(const TernaryExpression ternaryExpression) + { + astInformation.ternaryColonLocations ~= ternaryExpression.colon.index; + } + private: ASTInformation* astInformation; } diff --git a/src/dfmt/formatter.d b/src/dfmt/formatter.d index 8ea8cc6..d2c0f97 100644 --- a/src/dfmt/formatter.d +++ b/src/dfmt/formatter.d @@ -841,6 +841,7 @@ private: current.line); immutable bool isStructInitializer = astInformation.structInfoSortedByEndLocation .canFind!(st => st.startLocation < current.index && current.index < st.endLocation); + immutable bool isTernary = astInformation.ternaryColonLocations.canFindIndex(current.index); if (isCase || isAttribute) { @@ -856,7 +857,7 @@ private: newline(); } } - else if (indents.topIs(tok!"]")) // Associative array + else if (indents.topIs(tok!"]") && !isTernary) // Associative array { write(config.dfmt_space_before_aa_colon ? " : " : ": "); ++index; diff --git a/tests/allman/issue0578.d.ref b/tests/allman/issue0578.d.ref new file mode 100644 index 0000000..67284f7 --- /dev/null +++ b/tests/allman/issue0578.d.ref @@ -0,0 +1,43 @@ +void f() +{ + auto t = true ? 1 : 0; + auto a = [true ? 1 : 0]; + auto aa1 = [0: true ? 1 : 0]; + auto aa2 = [0: true ? (false ? 1 : 2) : 3]; + + auto aa3 = [0: true ? false ? 1: 2 : 3]; + /+ + READ IF THIS TEST FAILS + + Bug in dparse: + (Formatting before fix issue 578) + int[int] aa3 = [0: true ? false ? 1: 2: 3]; + ^ + + EXPLANATION: + The marked colon is not is not recognized as a TernaryExpression by + dparse: + If you write a `writeln(ternaryExpression.colon.index)` in the overloaded + ASTInformation visit function, which should get called once for every + encountered ternary colon, only the second index is printed: + override void visit(const TernaryExpression ternaryExpression) { ... } + + This bug can be ignored (formatting error is localized and should be rarely + encountered). + + + FIX: + Should this get fixed by dparse or when the migration to dmd is completed, + the formatting in the .ref files can be updated to the correct one and this + comment can be removed. + + + Current formatting after applying fix issue 578: + auto aa3 = [0: true ? false ? 1: 2 : 3]; + ^ + + Correct formatting after fix dparse: + auto aa3 = [0: true ? false ? 1 : 2 : 3]; + ^ + +/ +} diff --git a/tests/issue0578.d b/tests/issue0578.d new file mode 100644 index 0000000..24e818f --- /dev/null +++ b/tests/issue0578.d @@ -0,0 +1,43 @@ +void f() +{ + auto t = true ? 1 : 0; + auto a = [true ? 1: 0]; + auto aa1 = [0: true ? 1: 0]; + auto aa2 = [0: true ? (false ? 1: 2): 3]; + + auto aa3 = [0: true ? false ? 1: 2: 3]; + /+ + READ IF THIS TEST FAILS + + Bug in dparse: + (Formatting before fix issue 578) + int[int] aa3 = [0: true ? false ? 1: 2: 3]; + ^ + + EXPLANATION: + The marked colon is not is not recognized as a TernaryExpression by + dparse: + If you write a `writeln(ternaryExpression.colon.index)` in the overloaded + ASTInformation visit function, which should get called once for every + encountered ternary colon, only the second index is printed: + override void visit(const TernaryExpression ternaryExpression) { ... } + + This bug can be ignored (formatting error is localized and should be rarely + encountered). + + + FIX: + Should this get fixed by dparse or when the migration to dmd is completed, + the formatting in the .ref files can be updated to the correct one and this + comment can be removed. + + + Current formatting after applying fix issue 578: + auto aa3 = [0: true ? false ? 1: 2 : 3]; + ^ + + Correct formatting after fix dparse: + auto aa3 = [0: true ? false ? 1 : 2 : 3]; + ^ + +/ +} diff --git a/tests/knr/issue0578.d.ref b/tests/knr/issue0578.d.ref new file mode 100644 index 0000000..67284f7 --- /dev/null +++ b/tests/knr/issue0578.d.ref @@ -0,0 +1,43 @@ +void f() +{ + auto t = true ? 1 : 0; + auto a = [true ? 1 : 0]; + auto aa1 = [0: true ? 1 : 0]; + auto aa2 = [0: true ? (false ? 1 : 2) : 3]; + + auto aa3 = [0: true ? false ? 1: 2 : 3]; + /+ + READ IF THIS TEST FAILS + + Bug in dparse: + (Formatting before fix issue 578) + int[int] aa3 = [0: true ? false ? 1: 2: 3]; + ^ + + EXPLANATION: + The marked colon is not is not recognized as a TernaryExpression by + dparse: + If you write a `writeln(ternaryExpression.colon.index)` in the overloaded + ASTInformation visit function, which should get called once for every + encountered ternary colon, only the second index is printed: + override void visit(const TernaryExpression ternaryExpression) { ... } + + This bug can be ignored (formatting error is localized and should be rarely + encountered). + + + FIX: + Should this get fixed by dparse or when the migration to dmd is completed, + the formatting in the .ref files can be updated to the correct one and this + comment can be removed. + + + Current formatting after applying fix issue 578: + auto aa3 = [0: true ? false ? 1: 2 : 3]; + ^ + + Correct formatting after fix dparse: + auto aa3 = [0: true ? false ? 1 : 2 : 3]; + ^ + +/ +} diff --git a/tests/otbs/issue0578.d.ref b/tests/otbs/issue0578.d.ref new file mode 100644 index 0000000..1dbb893 --- /dev/null +++ b/tests/otbs/issue0578.d.ref @@ -0,0 +1,42 @@ +void f() { + auto t = true ? 1 : 0; + auto a = [true ? 1 : 0]; + auto aa1 = [0: true ? 1 : 0]; + auto aa2 = [0: true ? (false ? 1 : 2) : 3]; + + auto aa3 = [0: true ? false ? 1: 2 : 3]; + /+ + READ IF THIS TEST FAILS + + Bug in dparse: + (Formatting before fix issue 578) + int[int] aa3 = [0: true ? false ? 1: 2: 3]; + ^ + + EXPLANATION: + The marked colon is not is not recognized as a TernaryExpression by + dparse: + If you write a `writeln(ternaryExpression.colon.index)` in the overloaded + ASTInformation visit function, which should get called once for every + encountered ternary colon, only the second index is printed: + override void visit(const TernaryExpression ternaryExpression) { ... } + + This bug can be ignored (formatting error is localized and should be rarely + encountered). + + + FIX: + Should this get fixed by dparse or when the migration to dmd is completed, + the formatting in the .ref files can be updated to the correct one and this + comment can be removed. + + + Current formatting after applying fix issue 578: + auto aa3 = [0: true ? false ? 1: 2 : 3]; + ^ + + Correct formatting after fix dparse: + auto aa3 = [0: true ? false ? 1 : 2 : 3]; + ^ + +/ +}