From df4bbd4a66a84e8aaacaedc9277b74841e4d3bf8 Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Tue, 1 Nov 2016 15:24:38 +0300 Subject: [PATCH] PY-21244 Properly enumerate nested fields without explicit name or index Previously they were enumerated in the opposite order, i.e. descendant fields had lower indexes than their parents. Existing tests didn't caught that problem because I misinterpreted the method UsefulTestCase#assertSameElements(), should have used UsefulTestCase#assertOrderedEquals() instead. --- .../PyNewStyleStringFormatParser.java | 34 +++++----- .../formatMethodNestedFields3.py | 1 + .../formatMethodNestedFields3_after.py | 1 + .../python/PyStringFormatParserTest.java | 66 +++++++++---------- .../PyConvertToFStringIntentionTest.java | 5 ++ 5 files changed, 58 insertions(+), 49 deletions(-) create mode 100644 python/testData/intentions/PyConvertToFStringIntentionTest/formatMethodNestedFields3.py create mode 100644 python/testData/intentions/PyConvertToFStringIntentionTest/formatMethodNestedFields3_after.py diff --git a/python/src/com/jetbrains/python/inspections/PyNewStyleStringFormatParser.java b/python/src/com/jetbrains/python/inspections/PyNewStyleStringFormatParser.java index 0ab8b22e21c5..4b0049255781 100644 --- a/python/src/com/jetbrains/python/inspections/PyNewStyleStringFormatParser.java +++ b/python/src/com/jetbrains/python/inspections/PyNewStyleStringFormatParser.java @@ -117,6 +117,8 @@ public class PyNewStyleStringFormatParser { private Field parseField(int startOffset, int recursionDepth) { assert myNodeText.charAt(startOffset) == '{'; + int autoFieldNumber = myImplicitlyNumberedFieldsCounter; + // in the order of appearance inside a field final TIntArrayList attrAndLookupBounds = new TIntArrayList(); int conversionStart = -1; @@ -155,6 +157,11 @@ public class PyNewStyleStringFormatParser { if (!recovering) { // avoid duplicate offsets in sequences like "]." or "][" addIfNotLastItem(attrAndLookupBounds, offset); + + // no name in the field, increment implicitly named fields counter + if (attrAndLookupBounds.size() == 1 && attrAndLookupBounds.get(0) == startOffset + 1) { + myImplicitlyNumberedFieldsCounter++; + } } if (c == ':') { @@ -194,23 +201,18 @@ public class PyNewStyleStringFormatParser { addIfNotLastItem(attrAndLookupBounds, contentEnd); } - - final Field field = new Field(myNodeText, - startOffset, - attrAndLookupBounds.toNativeArray(), - conversionStart, - formatSpecStart, - nestedFields, - rightBraceOffset, - rightBraceOffset == -1 ? contentEnd : rightBraceOffset + 1, - myImplicitlyNumberedFieldsCounter, - recursionDepth); - assert !attrAndLookupBounds.isEmpty(); - if (attrAndLookupBounds.get(0) == startOffset + 1) { - myImplicitlyNumberedFieldsCounter++; - } - return field; + + return new Field(myNodeText, + startOffset, + attrAndLookupBounds.toNativeArray(), + conversionStart, + formatSpecStart, + nestedFields, + rightBraceOffset, + rightBraceOffset == -1 ? contentEnd : rightBraceOffset + 1, + autoFieldNumber, + recursionDepth); } private static void addIfNotLastItem(TIntArrayList attrAndLookupBounds, int offset) { diff --git a/python/testData/intentions/PyConvertToFStringIntentionTest/formatMethodNestedFields3.py b/python/testData/intentions/PyConvertToFStringIntentionTest/formatMethodNestedFields3.py new file mode 100644 index 000000000000..e934c6fa94d0 --- /dev/null +++ b/python/testData/intentions/PyConvertToFStringIntentionTest/formatMethodNestedFields3.py @@ -0,0 +1 @@ +'{:.{}}'.format(3.1415926, 3) \ No newline at end of file diff --git a/python/testData/intentions/PyConvertToFStringIntentionTest/formatMethodNestedFields3_after.py b/python/testData/intentions/PyConvertToFStringIntentionTest/formatMethodNestedFields3_after.py new file mode 100644 index 000000000000..7329a20ac086 --- /dev/null +++ b/python/testData/intentions/PyConvertToFStringIntentionTest/formatMethodNestedFields3_after.py @@ -0,0 +1 @@ +f'{3.1415926:.{3}}' \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/PyStringFormatParserTest.java b/python/testSrc/com/jetbrains/python/PyStringFormatParserTest.java index 1bf3e1968d00..ce177548de2e 100644 --- a/python/testSrc/com/jetbrains/python/PyStringFormatParserTest.java +++ b/python/testSrc/com/jetbrains/python/PyStringFormatParserTest.java @@ -178,8 +178,8 @@ public class PyStringFormatParserTest extends UsefulTestCase { final List topLevelFields = result.getFields(); assertSize(2, topLevelFields); assertSize(5, result.getAllFields()); - assertSameElements(result.getAllFields().stream().map(f -> f.getDepth()).toArray(), 1, 2, 3, 4, 1); - assertSameElements(result.getAllFields().stream().map(f -> f.getAutoPosition()).toArray(), 0, 1, 2, 3, 4); + assertOrderedEquals(result.getAllFields().stream().map(f -> f.getDepth()).toArray(), 1, 2, 3, 4, 1); + assertOrderedEquals(result.getAllFields().stream().map(f -> f.getAutoPosition()).toArray(), 0, 1, 2, 3, 4); } public void testNewStyleAttrAndLookups() { @@ -195,40 +195,40 @@ public class PyStringFormatParserTest extends UsefulTestCase { field = doParseAndGetFirstField("u'{foo.bar.baz}'"); assertEquals("foo", field.getFirstName()); - assertSameElements(field.getAttributesAndLookups(), ".bar", ".baz"); + assertOrderedEquals(field.getAttributesAndLookups(), ".bar", ".baz"); field = doParseAndGetFirstField("u'{foo[bar][baz]}'"); assertEquals("foo", field.getFirstName()); - assertSameElements(field.getAttributesAndLookups(), "[bar]", "[baz]"); + assertOrderedEquals(field.getAttributesAndLookups(), "[bar]", "[baz]"); field = doParseAndGetFirstField("u'{foo.bar[baz]}'"); assertEquals("foo", field.getFirstName()); - assertSameElements(field.getAttributesAndLookups(), ".bar", "[baz]"); + assertOrderedEquals(field.getAttributesAndLookups(), ".bar", "[baz]"); field = doParseAndGetFirstField("u'{foo.bar[baz}'"); assertEquals("foo", field.getFirstName()); - assertSameElements(field.getAttributesAndLookups(), ".bar"); + assertOrderedEquals(field.getAttributesAndLookups(), ".bar"); field = doParseAndGetFirstField("u'{foo[{bar[baz]}'"); assertEquals("foo", field.getFirstName()); - assertSameElements(field.getAttributesAndLookups(), "[{bar[baz]"); + assertOrderedEquals(field.getAttributesAndLookups(), "[{bar[baz]"); field = doParseAndGetFirstField("u'{foo[{} {0} {bar.baz}]'"); assertEquals("foo", field.getFirstName()); - assertSameElements(field.getAttributesAndLookups(), "[{} {0} {bar.baz}]"); + assertOrderedEquals(field.getAttributesAndLookups(), "[{} {0} {bar.baz}]"); field = doParseAndGetFirstField("u'{foo[bar]baz'"); assertEquals("foo", field.getFirstName()); - assertSameElements(field.getAttributesAndLookups(), "[bar]"); + assertOrderedEquals(field.getAttributesAndLookups(), "[bar]"); field = doParseAndGetFirstField("'{0[foo][.!:][}]}'"); assertEquals("0", field.getFirstName()); - assertSameElements(field.getAttributesAndLookups(), "[foo]", "[.!:]", "[}]"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", "[.!:]", "[}]"); field = doParseAndGetFirstField("'{.foo.bar}'"); assertEmpty(field.getFirstName()); assertEquals(TextRange.create(2, 2), field.getFirstNameRange()); - assertSameElements(field.getAttributesAndLookups(), ".foo", ".bar"); + assertOrderedEquals(field.getAttributesAndLookups(), ".foo", ".bar"); field = doParseAndGetFirstField("'{}'"); assertEmpty(field.getFirstName()); @@ -261,68 +261,68 @@ public class PyStringFormatParserTest extends UsefulTestCase { assertEmpty(field.getAttributesAndLookups()); field = doParseAndGetFirstField("'{0[foo].bar[baz]}'"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); field = doParseAndGetFirstField("'{0[foo].bar[baz]'"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); field = doParseAndGetFirstField("'{0[foo].bar[baz]"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); // do not recover unfinished lookups field = doParseAndGetFirstField("'{0[foo].bar[ba}'"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar"); field = doParseAndGetFirstField("'{0[foo].bar[ba!}'"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar"); field = doParseAndGetFirstField("'{0[foo].bar[ba:}'"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar"); field = doParseAndGetFirstField("'{0[foo].bar[ba'"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar"); field = doParseAndGetFirstField("'{0[foo].bar[ba"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar"); // do not recover illegal attributes field = doParseAndGetFirstField("'{0[foo].bar[baz]quux}'"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); field = doParseAndGetFirstField("'{0[foo].bar[baz]quux!}'"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); field = doParseAndGetFirstField("'{0[foo].bar[baz]quux:}'"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); field = doParseAndGetFirstField("'{0[foo].bar[baz]quux'"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); field = doParseAndGetFirstField("'{0[foo].bar[baz]quux"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar", "[baz]"); field = doParseAndGetFirstField("'{0..}'"); assertEquals("0", field.getFirstName()); - assertSameElements(field.getAttributesAndLookups(), ".", "."); + assertOrderedEquals(field.getAttributesAndLookups(), ".", "."); // recover attributes field = doParseAndGetFirstField("'{0[foo].}'"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", "."); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", "."); field = doParseAndGetFirstField("'{0[foo].'"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", "."); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", "."); field = doParseAndGetFirstField("'{0[foo]."); - assertSameElements(field.getAttributesAndLookups(), "[foo]", "."); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", "."); field = doParseAndGetFirstField("'{0[foo].bar}'"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar"); field = doParseAndGetFirstField("'{0[foo].bar'"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar"); field = doParseAndGetFirstField("'{0[foo].bar"); - assertSameElements(field.getAttributesAndLookups(), "[foo]", ".bar"); + assertOrderedEquals(field.getAttributesAndLookups(), "[foo]", ".bar"); } public void testAutoPosition() { @@ -361,12 +361,12 @@ public class PyStringFormatParserTest extends UsefulTestCase { public void testNewStyleNamedUnicodeEscapeInLookup() { final Field field = doParseAndGetFirstField("'{foo[\\N{ESCAPE WITH ]}]}'"); - assertSameElements(field.getAttributesAndLookups(), "[\\N{ESCAPE WITH ]}]"); + assertOrderedEquals(field.getAttributesAndLookups(), "[\\N{ESCAPE WITH ]}]"); } public void testNewStyleNamedUnicodeEscapeInAttribute() { final Field field = doParseAndGetFirstField("'{foo.b\\N{ESCAPE WITH [}.b\\N{ESCAPE WITH .}}'"); - assertSameElements(field.getAttributesAndLookups(), ".b\\N{ESCAPE WITH [}", ".b\\N{ESCAPE WITH .}"); + assertOrderedEquals(field.getAttributesAndLookups(), ".b\\N{ESCAPE WITH [}", ".b\\N{ESCAPE WITH .}"); } public void testNewStyleUnclosedLookupEndsWithRightBrace() { diff --git a/python/testSrc/com/jetbrains/python/intentions/PyConvertToFStringIntentionTest.java b/python/testSrc/com/jetbrains/python/intentions/PyConvertToFStringIntentionTest.java index 71b67a9425ba..7d1f2528a489 100644 --- a/python/testSrc/com/jetbrains/python/intentions/PyConvertToFStringIntentionTest.java +++ b/python/testSrc/com/jetbrains/python/intentions/PyConvertToFStringIntentionTest.java @@ -158,4 +158,9 @@ public class PyConvertToFStringIntentionTest extends PyIntentionTestCase { public void testFormatMethodNestedFields2() { doTest(); } + + // PY-21244 + public void testFormatMethodNestedFields3() { + doTest(); + } }