[PY-27601] Updated fix for "Too few arguments for format string"

There are two important changes:

Based on the implementation at https://github.com/python/cpython/blob/master/Objects/stringlib/unicode_format.h#L82 I have added two new warning messages to PyStringFormatInspection, which detect changes between manual and automatic field numbering.

I have also fixed the way PySubstitutionChunkReference uses myPosition
field to refer to the correct positional argument. It uses the explicitly specified position, if present, or the auto-numbered one otherwise. I have tried to fix all the visible regressions introduced by this change.
This commit is contained in:
Andrei-Silviu DRAGNEA (24887)
2018-04-24 19:12:36 +03:00
committed by Valentina Kiryushkina
parent 9a52af5553
commit 2a37853de1
10 changed files with 76 additions and 20 deletions
@@ -395,6 +395,8 @@ INSP.too.few.args.for.fmt.string=Too few arguments for format string
INSP.incompatible.options=The format options in chunk "{0}" are incompatible
INSP.unused.mapping = Mapping key "{0}" is unused
INSP.unsupported.format.character=Unsupported format character ''{0}''
INSP.manual.to.auto.field.numbering=Cannot switch from manual field specification to automatic field numbering
INSP.auto.to.manual.field.numbering=Cannot switch from automatic field numbering to manual field specification
# PyMethodOverridingInspection
INSP.NAME.method.over=Method signature does not match signature of overridden method
@@ -74,7 +74,6 @@ public class PySubstitutionChunkReference extends PsiReferenceBase<PyStringLiter
@Nullable
private PsiElement resolvePositionalFormat(@NotNull PyArgumentList argumentList) {
final int position = myChunk.getAutoPosition() == null ? myPosition : myChunk.getAutoPosition();
int n = 0;
boolean notSureAboutStarArgs = false;
PyStarArgument firstStarArg = null;
@@ -99,7 +98,7 @@ public class PySubstitutionChunkReference extends PsiReferenceBase<PyStringLiter
}
}
else if (!(arg instanceof PyKeywordArgument)) {
if (position == n) {
if (myPosition == n) {
return arg;
}
n++;
@@ -56,7 +56,7 @@ public class PythonFormattedStringReferenceProvider extends PsiReferenceProvider
final PySubstitutionChunkReference[] result = new PySubstitutionChunkReference[chunks.size()];
for (int i = 0; i < chunks.size(); i++) {
final PyStringFormatParser.SubstitutionChunk chunk = chunks.get(i);
result[i] = new PySubstitutionChunkReference(element, chunk, i);
result[i] = new PySubstitutionChunkReference(element, chunk, chunk.getPositionalArgumentIndex());
}
return result;
}
@@ -19,7 +19,6 @@ import com.intellij.openapi.util.TextRange;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.psi.PsiElement;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.ObjectUtils;
import com.jetbrains.python.codeInsight.PySubstitutionChunkReference;
import com.jetbrains.python.inspections.PyNewStyleStringFormatParser;
import com.jetbrains.python.inspections.PyNewStyleStringFormatParser.Field;
@@ -53,7 +52,7 @@ public class NewStyleConvertToFStringProcessor extends BaseConvertToFStringProce
@NotNull
@Override
protected PySubstitutionChunkReference createReference(@NotNull Field field) {
return new PySubstitutionChunkReference(myPyString, field, ObjectUtils.chooseNotNull(field.getAutoPosition(), 0));
return new PySubstitutionChunkReference(myPyString, field, field.getPositionalArgumentIndex());
}
@Override
@@ -10,7 +10,6 @@ import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiElementVisitor;
import com.intellij.psi.PsiFile;
import com.intellij.psi.util.PsiTreeUtil;
import java.util.HashMap;
import com.jetbrains.python.PyBundle;
import com.jetbrains.python.PyNames;
import com.jetbrains.python.codeInsight.PySubstitutionChunkReference;
@@ -480,16 +479,25 @@ public class PyStringFormatInspection extends PyInspection {
public void inspect() {
final String value = myFormatExpression.getText();
final List<PyStringFormatParser.SubstitutionChunk> chunks = filterSubstitutions(PyStringFormatParser.parseNewStyleFormat(value));
PyStringFormatParser parser = new PyStringFormatParser(value);
final List<PyStringFormatParser.SubstitutionChunk> chunks = filterSubstitutions(parser.parseNewStyle());
switch (parser.getAutoNumberStateError()) {
case NONE:
break;
case MANUAL_TO_AUTO:
registerProblem(myFormatExpression, PyBundle.message("INSP.manual.to.auto.field.numbering"));
return;
case AUTO_TO_MANUAL:
registerProblem(myFormatExpression, PyBundle.message("INSP.auto.to.manual.field.numbering"));
return;
}
for (int i = 0; i < chunks.size(); i++) {
final PyStringFormatParser.NewStyleSubstitutionChunk chunk =
as(chunks.get(i), PyStringFormatParser.NewStyleSubstitutionChunk.class);
if (chunk != null) {
if (chunk.getPosition() == null) {
chunk.setPosition(i);
}
String mappingKey = inspectNewStyleChunkAndGetMappingKey(chunk);
if (!isProblem()) {
inspectArguments(chunk, mappingKey);
@@ -502,7 +510,7 @@ public class PyStringFormatInspection extends PyInspection {
final HashSet<String> supportedTypes = new HashSet<>();
boolean hasTypeOptions = false;
final String mappingKey = chunk.getMappingKey() != null ? chunk.getMappingKey() : String.valueOf(chunk.getPosition());
final String mappingKey = chunk.getMappingKey() != null ? chunk.getMappingKey() : String.valueOf(chunk.getPositionalArgumentIndex());
// inspect options available only for numeric types
if (chunk.hasSignOption() || chunk.useAlternateForm() || chunk.hasZeroPadding() || chunk.hasThousandsSeparator()) {
@@ -539,9 +547,7 @@ public class PyStringFormatInspection extends PyInspection {
}
private void inspectArguments(@NotNull PyStringFormatParser.NewStyleSubstitutionChunk chunk, @NotNull String mappingKey) {
// it's true because we set position manually in inspect()
assert chunk.getPosition() != null;
final PsiElement target = new PySubstitutionChunkReference(myFormatExpression, chunk, chunk.getPosition()).resolve();
final PsiElement target = new PySubstitutionChunkReference(myFormatExpression, chunk, chunk.getPositionalArgumentIndex()).resolve();
boolean hasElementIndex = chunk.getMappingKeyElementIndex() != null;
if (target == null) {
final String chunkMapping = chunk.getMappingKey();
@@ -18,7 +18,6 @@ package com.jetbrains.python.inspections;
import com.intellij.openapi.util.TextRange;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.psi.PsiElement;
import java.util.HashMap;
import com.jetbrains.python.PyNames;
import com.jetbrains.python.psi.*;
import com.jetbrains.python.psi.impl.PyStringLiteralExpressionImpl;
@@ -26,6 +25,7 @@ import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.regex.Matcher;
@@ -144,6 +144,10 @@ public class PyStringFormatParser {
protected void setMappingKey(@Nullable String mappingKey) {
myMappingKey = mappingKey;
}
public int getPositionalArgumentIndex() {
return myPosition == null ? myAutoPosition == null ? 0 : myAutoPosition : myPosition;
}
}
public static class PercentSubstitutionChunk extends SubstitutionChunk {
@@ -254,10 +258,43 @@ public class PyStringFormatParser {
}
}
private enum AutoNumberState {INIT, AUTO, MANUAL}
public enum AutoNumberStateError {NONE, MANUAL_TO_AUTO, AUTO_TO_MANUAL}
private static class AutoNumber {
private AutoNumberState myState = AutoNumberState.INIT;
private AutoNumberStateError myStateError = AutoNumberStateError.NONE;
private void checkState(AutoNumberState nextState) {
if (myStateError != AutoNumberStateError.NONE) {
return;
}
switch (myState) {
case INIT:
myState = nextState;
break;
case AUTO:
if (nextState == AutoNumberState.MANUAL) {
myStateError = AutoNumberStateError.AUTO_TO_MANUAL;
}
break;
case MANUAL:
if (nextState == AutoNumberState.AUTO) {
myStateError = AutoNumberStateError.MANUAL_TO_AUTO;
}
break;
}
}
}
public AutoNumberStateError getAutoNumberStateError() {
return myAutoNumber.myStateError;
}
@NotNull private final String myLiteral;
@NotNull private final List<FormatStringChunk> myResult = new ArrayList<>();
private int myPos;
private int mySubstitutionsCount = 0;
@NotNull private AutoNumber myAutoNumber = new AutoNumber();
// % strings
private static final String CONVERSION_FLAGS = "#0- +";
@@ -273,7 +310,7 @@ public class PyStringFormatParser {
private static final char ZERO_PADDING_SYMBOL = '0';
private PyStringFormatParser(@NotNull String literal) {
public PyStringFormatParser(@NotNull String literal) {
myLiteral = literal;
}
@@ -308,7 +345,7 @@ public class PyStringFormatParser {
return myResult;
}
private List<FormatStringChunk> parseNewStyle() {
public List<FormatStringChunk> parseNewStyle() {
final List<FormatStringChunk> results = new ArrayList<>();
final Matcher matcher = NEW_STYLE_FORMAT_TOKENS.matcher(myLiteral);
int autoPositionedFieldsCount = 0;
@@ -350,6 +387,7 @@ public class PyStringFormatParser {
try {
final int number = Integer.parseInt(name);
chunk.setPosition(number);
myAutoNumber.checkState(AutoNumberState.MANUAL);
}
catch (NumberFormatException e) {
chunk.setMappingKey(name);
@@ -359,6 +397,7 @@ public class PyStringFormatParser {
else {
chunk.setAutoPosition(autoPositionedFieldsCount);
autoPositionedFieldsCount++;
myAutoNumber.checkState(AutoNumberState.AUTO);
}
// parse field name attribute name
@@ -453,7 +492,7 @@ public class PyStringFormatParser {
if (myPos < end - 1) {
chunk.setConversionType(myLiteral.charAt(myPos));
}
}
}
results.add(chunk);
@@ -0,0 +1 @@
<warning descr="Cannot switch from automatic field numbering to manual field specification">'{} {1}'</warning>.format(6, 7)
@@ -0,0 +1 @@
<warning descr="Cannot switch from manual field specification to automatic field numbering">'{1} {}'</warning>.format(6, 7)
@@ -1 +1,2 @@
'{a} {}'.format(6, a=2)
x = '{a} {}'.format(6, a=2)
print("{} {other} {}".format("one", "two", other="OTHER"))
@@ -125,10 +125,18 @@ public class PyStringFormatInspectionTest extends PyInspectionTestCase {
doTest();
}
// PY-27710
// PY-27601
public void testNewStylePositionalSubstitutionAfterKeywordSubstitution() {
doTest();
}
public void testNewStyleAutomaticAfterManualNumbering() {
doTest();
}
public void testNewStyleManualAfterAutomaticNumbering() {
doTest();
}
public void testPercentStringWithFormatStringReplacementSymbols() {
doTest();