Add rule: truthy, to forbid truthy values that are not quoted. #22

pdericson · 2016-09-28T12:04:23Z

No description provided.

coveralls · 2016-09-28T12:11:23Z

Coverage increased (+0.02%) to 98.504% when pulling 27f5470 on pdericson:truthy into c163135 on adrienverge:master.

coveralls · 2016-09-28T12:11:23Z

Coverage increased (+0.02%) to 98.504% when pulling 27f5470 on pdericson:truthy into c163135 on adrienverge:master.

pdericson · 2016-09-28T12:12:24Z

tests/test_cli.py

@@ -279,7 +279,7 @@ def test_run_non_ascii_file(self):
        # Make sure the default localization conditions on this "system"
        # support UTF-8 encoding.
        loc = locale.getlocale()
-        locale.setlocale(locale.LC_ALL, 'C.UTF-8')
+        locale.setlocale(locale.LC_ALL, 'en_US.UTF-8')


C.UTF-8 didn't work for me on OS X...

Please don't include this is this pull-request, we can do another one if needed.

(By the way en_US.UTF-8 isn't supposed to be present on non-English-speaking systems, and I know it isn't on Debian build system.)

I have reverted this change, I can run the tests in docker so all good

docker run -v $PWD:/mnt -w /mnt python:2.7 python setup.py test

adrienverge

This looks good, thanks @pdericson!

I just have a few remarks.

adrienverge · 2016-09-29T20:25:29Z

tests/test_cli.py

@@ -279,7 +279,7 @@ def test_run_non_ascii_file(self):
        # Make sure the default localization conditions on this "system"
        # support UTF-8 encoding.
        loc = locale.getlocale()
-        locale.setlocale(locale.LC_ALL, 'C.UTF-8')
+        locale.setlocale(locale.LC_ALL, 'en_US.UTF-8')


Please don't include this is this pull-request, we can do another one if needed.

(By the way en_US.UTF-8 isn't supposed to be present on non-English-speaking systems, and I know it isn't on Debian build system.)

adrienverge · 2016-09-29T20:26:30Z

yamllint/conf/relaxed.yaml

@@ -26,3 +26,5 @@ rules:
  line-length:
    level: warning
    allow-non-breakable-inline-mappings: yes
+  truthy:
+    level: warning


I'd rather have it disabled in relaxed conf. I know a bunch of people that are really attached to their yes/no in Ansible...

adrienverge · 2016-09-29T20:27:00Z

tests/rules/test_truthy.py

@@ -0,0 +1,33 @@
+# -*- coding: utf-8 -*-
+# Copyright (C) 2016 Adrien Vergé


Hey! I didn't write this. ;)

adrienverge · 2016-09-29T20:27:16Z

yamllint/rules/truthy.py

@@ -0,0 +1,56 @@
+# -*- coding: utf-8 -*-
+# Copyright (C) 2016 Adrien Vergé


Please put your name here :)

adrienverge · 2016-09-29T20:34:35Z

tests/test_spec_examples.py

@@ -119,6 +120,9 @@ class SpecificationTestCase(RuleTestCase):
    'example-8.22': ('indentation: disable\n'),
    'example-10.1': ('colons: {max-spaces-before: 2}\n'),
    'example-10.2': ('indentation: {indent-sequences: no}\n'),
+    'example-10.5': ('truthy: disable\n'),


Why this one?

adrienverge · 2016-09-29T20:34:39Z

tests/test_spec_examples.py

@@ -65,6 +65,7 @@ class SpecificationTestCase(RuleTestCase):
    'example-2.16': ('empty-lines: {max-end: 1}\n'),
    'example-2.18': ('empty-lines: {max-end: 1}\n'),
    'example-2.19': ('empty-lines: {max-end: 1}\n'),
+    'example-2.21': ('truthy: disable\n'),


Why this one?

adrienverge · 2016-09-29T20:46:36Z

yamllint/rules/truthy.py

+    if isinstance(token, yaml.tokens.ScalarToken):
+        if token.value in TRUTHY and token.style is None:
+            yield LintProblem(token.start_mark.line + 1,
+                              max(1, token.start_mark.column),


This should be token.start_mark.column + 1. And why max()?

I copied this from commas.py, I can see why it's invalid now :)

adrienverge · 2016-09-29T20:47:25Z

tests/rules/test_truthy.py

+        self.check('---\n1: True\n', conf, problem=(2, 3))
+        self.check('---\nTrue: 1\n', conf, problem=(2, 1))
+        self.check('---\n1: "True"\n', conf)
+        self.check('---\n"True": 1\n', conf)


Could you add this?

self.check('---\n' '!!seq [\n' ' !!bool true, !!bool false,\n' ' !!bool "false", !!bool "FALSE",\n' ' !!bool "true", !!bool "True",\n' ' !!bool "false", !!bool "FALSE",\n' ']\n', conf) self.check('---\n' '!!seq [\n' ' !!bool true, !!bool True,\n' ']\n', conf, problem=(3, 23))

coveralls · 2016-09-29T23:06:55Z

Coverage increased (+0.02%) to 98.504% when pulling a643c85 on pdericson:truthy into c163135 on adrienverge:master.

coveralls · 2016-09-29T23:09:48Z

Coverage increased (+0.02%) to 98.504% when pulling bc3ed94 on pdericson:truthy into c163135 on adrienverge:master.

coveralls · 2016-09-29T23:11:12Z

Coverage increased (+0.02%) to 98.504% when pulling 1576338 on pdericson:truthy into c163135 on adrienverge:master.

coveralls · 2016-09-29T23:22:29Z

Coverage increased (+0.02%) to 98.504% when pulling 843eec5 on pdericson:truthy into c163135 on adrienverge:master.

adrienverge · 2016-09-30T16:17:50Z

That's better, thanks!

Have you got a chance to consider my other remarks? (the extra tests with !!bool and the spec examples you disable -- I think two of them are working fine and shouldn't be disabled)

coveralls · 2016-10-01T00:03:38Z

Coverage increased (+0.02%) to 98.504% when pulling 475d316 on pdericson:truthy into c163135 on adrienverge:master.

pdericson · 2016-10-01T00:05:47Z

tests/rules/test_truthy.py

+                   ']\n', conf)
+        self.check('---\n'
+                   '!!seq [\n'
+                   '  !!bool true, !!bool True,\n'


@adrienverge I'm wondering if !!bool True should be allowed by the truthy rule. I haven't figured out yet if the explicit tag type is available to inspect or is lost during parsing by pyyaml...

pdericson · 2016-10-01T00:09:58Z

Sorry, my session ended abruptly yesterday, I was addressing feedback during my commute

coveralls · 2016-10-01T07:00:57Z

Coverage increased (+0.02%) to 98.504% when pulling f4c522d on pdericson:truthy into c163135 on adrienverge:master.

coveralls · 2016-10-01T07:01:59Z

Coverage increased (+0.02%) to 98.504% when pulling 62fe116 on pdericson:truthy into c163135 on adrienverge:master.

@adrienverge

From @adrienverge

adrienverge · 2016-10-01T07:06:06Z

@pdericson this is great, thanks for your contribution!

coveralls · 2016-10-01T07:06:29Z

Coverage increased (+0.02%) to 98.504% when pulling 5294ff5 on pdericson:truthy into c163135 on adrienverge:master.

ghiculescu · 2017-11-03T01:34:43Z

just a quick heads up that the version of yamllint that apt-get installs doesnt include this rule. the pip version does.

adrienverge · 2017-11-03T10:15:47Z

@ghiculescu Are you using Debian? Ubuntu?

@debalance is kindly maintaining packages for Debian, which means new versions should also end up in next Ubuntu versions. For really up-to-date versions, we would need an Ubuntu maintainer (help wanted!)

pdericson mentioned this pull request Sep 28, 2016

Add the ability to run custom rules included in a project #19

Closed

pdericson commented Sep 28, 2016

View reviewed changes

adrienverge requested changes Sep 29, 2016

View reviewed changes

pdericson commented Oct 1, 2016

View reviewed changes

pdericson added 2 commits October 1, 2016 09:03

Add rule: truthy, to forbid truthy values that are not quoted

1f472bc

truthy: Add tests for explicit booleans

5294ff5

From @adrienverge

adrienverge approved these changes Oct 1, 2016

View reviewed changes

adrienverge merged commit ff27f88 into adrienverge:master Oct 1, 2016

pdericson deleted the truthy branch October 3, 2016 21:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add rule: truthy, to forbid truthy values that are not quoted. #22

Add rule: truthy, to forbid truthy values that are not quoted. #22

pdericson commented Sep 28, 2016

coveralls commented Sep 28, 2016 •

edited

Loading

coveralls commented Sep 28, 2016

pdericson Sep 28, 2016

adrienverge Sep 29, 2016

pdericson Sep 29, 2016

adrienverge left a comment

adrienverge Sep 29, 2016

adrienverge Sep 29, 2016 •

edited

Loading

adrienverge Sep 29, 2016

adrienverge Sep 29, 2016

adrienverge Sep 29, 2016

adrienverge Sep 29, 2016

adrienverge Sep 29, 2016 •

edited

Loading

pdericson Sep 29, 2016

adrienverge Sep 29, 2016 •

edited

Loading

coveralls commented Sep 29, 2016 •

edited

Loading

coveralls commented Sep 29, 2016 •

edited

Loading

coveralls commented Sep 29, 2016 •

edited

Loading

coveralls commented Sep 29, 2016 •

edited

Loading

adrienverge commented Sep 30, 2016

coveralls commented Oct 1, 2016 •

edited

Loading

pdericson Oct 1, 2016

pdericson commented Oct 1, 2016

coveralls commented Oct 1, 2016 •

edited

Loading

coveralls commented Oct 1, 2016 •

edited

Loading

adrienverge commented Oct 1, 2016

coveralls commented Oct 1, 2016 •

edited

Loading

ghiculescu commented Nov 3, 2017

adrienverge commented Nov 3, 2017

		@@ -0,0 +1,33 @@
		# -- coding: utf-8 --
		# Copyright (C) 2016 Adrien Vergé

		@@ -0,0 +1,56 @@
		# -- coding: utf-8 --
		# Copyright (C) 2016 Adrien Vergé

Add rule: truthy, to forbid truthy values that are not quoted. #22

Add rule: truthy, to forbid truthy values that are not quoted. #22

Conversation

pdericson commented Sep 28, 2016

coveralls commented Sep 28, 2016 • edited Loading

coveralls commented Sep 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrienverge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrienverge Sep 29, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrienverge Sep 29, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrienverge Sep 29, 2016 • edited Loading

Choose a reason for hiding this comment

coveralls commented Sep 29, 2016 • edited Loading

coveralls commented Sep 29, 2016 • edited Loading

coveralls commented Sep 29, 2016 • edited Loading

coveralls commented Sep 29, 2016 • edited Loading

adrienverge commented Sep 30, 2016

coveralls commented Oct 1, 2016 • edited Loading

Choose a reason for hiding this comment

pdericson commented Oct 1, 2016

coveralls commented Oct 1, 2016 • edited Loading

coveralls commented Oct 1, 2016 • edited Loading

adrienverge commented Oct 1, 2016

coveralls commented Oct 1, 2016 • edited Loading

ghiculescu commented Nov 3, 2017

adrienverge commented Nov 3, 2017

coveralls commented Sep 28, 2016 •

edited

Loading

adrienverge Sep 29, 2016 •

edited

Loading

adrienverge Sep 29, 2016 •

edited

Loading

adrienverge Sep 29, 2016 •

edited

Loading

coveralls commented Sep 29, 2016 •

edited

Loading

coveralls commented Sep 29, 2016 •

edited

Loading

coveralls commented Sep 29, 2016 •

edited

Loading

coveralls commented Sep 29, 2016 •

edited

Loading

coveralls commented Oct 1, 2016 •

edited

Loading

coveralls commented Oct 1, 2016 •

edited

Loading

coveralls commented Oct 1, 2016 •

edited

Loading

coveralls commented Oct 1, 2016 •

edited

Loading