Skip to content

Commit

Permalink
quoted-strings: Fix only-when-needed on corner cases
Browse files Browse the repository at this point in the history
Change implementation of `required: only-when-needed`, because
maintaining a list of `START_TOKENS` and just looking at the first
character of string values has proven to be partially broken.

Cf. discussion at
#246 (comment).

Fixes #242.
  • Loading branch information
adrienverge committed Apr 13, 2020
1 parent 961c496 commit 7d42414
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 8 deletions.
39 changes: 39 additions & 0 deletions tests/rules/test_quoted_strings.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,3 +316,42 @@ def test_only_when_needed_single_quotes(self):
' "word 1\\\n' # fails
' word 2"\n',
conf, problem1=(12, 3))

def test_only_when_needed_corner_cases(self):
conf = 'quoted-strings: {required: only-when-needed}\n'

self.check('---\n'
'- "- item"\n'
'- "key: value"\n'
'- "%H:%M:%S"\n'
'- "%wheel ALL=(ALL) NOPASSWD: ALL"\n'
'- \'"quoted"\'\n'
'- "\'foo\' == \'bar\'"\n'
'- "\'Mac\' in ansible_facts.product_name"\n',
conf)
self.check('---\n'
'k1: "- item"\n'
'k2: "key: value"\n'
'k3: "%H:%M:%S"\n'
'k4: "%wheel ALL=(ALL) NOPASSWD: ALL"\n'
'k5: \'"quoted"\'\n'
'k6: "\'foo\' == \'bar\'"\n'
'k7: "\'Mac\' in ansible_facts.product_name"\n',
conf)

self.check('---\n'
'- ---\n'
'- "---"\n' # fails
'- ----------\n'
'- "----------"\n' # fails
'- :wq\n'
'- ":wq"\n', # fails
conf, problem1=(3, 3), problem2=(5, 3), problem3=(7, 3))
self.check('---\n'
'k1: ---\n'
'k2: "---"\n' # fails
'k3: ----------\n'
'k4: "----------"\n' # fails
'k5: :wq\n'
'k6: ":wq"\n', # fails
conf, problem1=(3, 5), problem2=(5, 5), problem3=(7, 5))
30 changes: 22 additions & 8 deletions yamllint/rules/quoted_strings.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,30 @@
'required': True}

DEFAULT_SCALAR_TAG = u'tag:yaml.org,2002:str'
START_TOKENS = {'#', '*', '!', '?', '@', '`', '&',
',', '-', '{', '}', '[', ']', ':'}


def quote_match(quote_type, token_style):
def _quote_match(quote_type, token_style):
return ((quote_type == 'any') or
(quote_type == 'single' and token_style == "'") or
(quote_type == 'double' and token_style == '"'))


def _quotes_are_needed(string):
loader = yaml.BaseLoader('key: ' + string)
# Remove the 5 first tokens corresponding to 'key: ' (StreamStartToken,
# BlockMappingStartToken, KeyToken, ScalarToken(value=key), ValueToken)
for _ in range(5):
loader.get_token()
try:
a, b = loader.get_token(), loader.get_token()
if (isinstance(a, yaml.ScalarToken) and a.style is None and
isinstance(b, yaml.BlockEndToken)):
return False
return True
except yaml.scanner.ScannerError:
return True


def check(conf, token, prev, next, nextnext, context):
if not (isinstance(token, yaml.tokens.ScalarToken) and
isinstance(prev, (yaml.BlockEntryToken, yaml.FlowEntryToken,
Expand Down Expand Up @@ -121,25 +135,25 @@ def check(conf, token, prev, next, nextnext, context):
if required is True:

# Quotes are mandatory and need to match config
if token.style is None or not quote_match(quote_type, token.style):
if token.style is None or not _quote_match(quote_type, token.style):
msg = "string value is not quoted with %s quotes" % (quote_type)

elif required is False:

# Quotes are not mandatory but when used need to match config
if token.style and not quote_match(quote_type, token.style):
if token.style and not _quote_match(quote_type, token.style):
msg = "string value is not quoted with %s quotes" % (quote_type)

elif not token.plain:

# Quotes are disallowed when not needed
if (tag == DEFAULT_SCALAR_TAG and token.value
and token.value[0] not in START_TOKENS):
if (tag == DEFAULT_SCALAR_TAG and token.value and
not _quotes_are_needed(token.value)):
msg = "string value is redundantly quoted with %s quotes" % (
quote_type)

# But when used need to match config
elif token.style and not quote_match(quote_type, token.style):
elif token.style and not _quote_match(quote_type, token.style):
msg = "string value is not quoted with %s quotes" % (quote_type)

if msg is not None:
Expand Down

0 comments on commit 7d42414

Please sign in to comment.