Merge pull request #301 from olehermanse/master

Improved validation error messages for module names

Author: olehermanse

cfbs/cfbs_config.py

@@ -13,7 +13,6 @@
   3. Add modules and make commits
 """
 
-
 import os
 import copy
 import glob

cfbs/git_magic.py

@@ -19,7 +19,6 @@
 import logging as log
 from functools import partial
 
-
 first_commit = True
 
 

cfbs/man_generator.py

@@ -18,9 +18,7 @@ def generate_man_page():
         + "Modules can be custom promise types, JSON files which enable certain functionality, or reusable CFEngine policy. "
         + "The modules you use can be written by the CFEngine team, others in the community, your colleagues, or yourself."
     )
-    body = (
-        str(manpage)
-        + """
+    body = str(manpage) + """
 .br
 Binary packages may be downloaded from https://cfengine.com/download/.
 .br
@@ -40,7 +38,6 @@ def generate_man_page():
 .SH AUTHOR
     Northern.tech AS
         """
-    )
     with open(os.path.dirname(__file__) + "/cfbs.1", "w", encoding="utf-8") as man_file:
         man_file.write(body)
     return body

cfbs/validate.py

@@ -227,6 +227,10 @@ def validate_module_name_content(name):
     r = "[a-z][a-z0-9]*(-[a-z0-9]+)*"
     proper_name = name
 
+    if is_module_local(name) or is_module_absolute(name):
+        # If one of these are true, the other must be false
+        assert is_module_absolute(name) != is_module_local(name)
+
     if is_module_local(name):
         if not name.startswith("./"):
             raise CFBSValidationError(name, "Local module names should begin with `./`")
@@ -270,10 +274,18 @@ def validate_module_name_content(name):
             "Module name proper is empty",
         )
 
+    if proper_name[0] not in "abcdefghijklmnopqrstuvwxyz":
+        raise CFBSValidationError(
+            name,
+            "Module name must start with a lowercase ASCII letter ('{}' starts with '{}')".format(
+                proper_name, proper_name[0]
+            ),
+        )
+
     if not re.fullmatch(r, proper_name):
         raise CFBSValidationError(
             name,
-            "Module name contains illegal characters (only lowercase ASCII alphanumeric characters are legal)",
+            "Module name contains illegal characters (only lowercase ASCII alphanumeric characters and single dash separators are allowed)",
         )
 
     log.debug("Successfully validated name of module %s" % name)

tests/test_pretty.py

@@ -220,15 +220,10 @@ def test_pretty_file():
 def test_pretty_check_string():
     assert pretty_check_string(' "Hello" ') is False
     assert pretty_check_string('"Hello"') is True
-    assert (
-        pretty_check_string(
-            """{
+    assert pretty_check_string("""{
   "name": "lars",
   "age": 27
-}"""
-        )
-        is True
-    )
+}""") is True
     assert pretty_check_string('{ "name": "lars", "age": 27 }') is False
 
 
@@ -242,29 +237,20 @@ def test_pretty_sorting_simple_top_level():
         ),
     }
     assert pretty_string("""{}""", lex_sorting) == """{}"""
-    assert (
-        pretty_string("""{"a":1}""", lex_sorting)
-        == """{
+    assert pretty_string("""{"a":1}""", lex_sorting) == """{
   "a": 1
 }"""
-    )
 
-    assert (
-        pretty_string("""{"b":2,"a":1}""", lex_sorting)
-        == """{
+    assert pretty_string("""{"b":2,"a":1}""", lex_sorting) == """{
   "a": 1,
   "b": 2
 }"""
-    )
 
-    assert (
-        pretty_string("""{"b":2,"a":1,"c":3}""", lex_sorting)
-        == """{
+    assert pretty_string("""{"b":2,"a":1,"c":3}""", lex_sorting) == """{
   "a": 1,
   "b": 2,
   "c": 3
 }"""
-    )
 
     length_sorting = {
         None: (
@@ -274,29 +260,20 @@ def test_pretty_sorting_simple_top_level():
     }
 
     assert pretty_string("""{}""", length_sorting) == """{}"""
-    assert (
-        pretty_string("""{"aa":1}""", length_sorting)
-        == """{
+    assert pretty_string("""{"aa":1}""", length_sorting) == """{
   "aa": 1
 }"""
-    )
 
-    assert (
-        pretty_string("""{"bbb":2,"aa":1}""", length_sorting)
-        == """{
+    assert pretty_string("""{"bbb":2,"aa":1}""", length_sorting) == """{
   "aa": 1,
   "bbb": 2
 }"""
-    )
 
-    assert (
-        pretty_string("""{"bbb":2,"aa":1,"c":3}""", length_sorting)
-        == """{
+    assert pretty_string("""{"bbb":2,"aa":1,"c":3}""", length_sorting) == """{
   "c": 3,
   "aa": 1,
   "bbb": 2
 }"""
-    )
 
     integer_sorting = {
         None: (
@@ -306,39 +283,27 @@ def test_pretty_sorting_simple_top_level():
     }
 
     assert pretty_string("""{}""", integer_sorting) == """{}"""
-    assert (
-        pretty_string("""{"a":1}""", integer_sorting)
-        == """{
+    assert pretty_string("""{"a":1}""", integer_sorting) == """{
   "a": 1
 }"""
-    )
 
-    assert (
-        pretty_string("""{"b":2,"a":1}""", integer_sorting)
-        == """{
+    assert pretty_string("""{"b":2,"a":1}""", integer_sorting) == """{
   "a": 1,
   "b": 2
 }"""
-    )
 
-    assert (
-        pretty_string("""{"b":2,"a":1,"c":3}""", integer_sorting)
-        == """{
+    assert pretty_string("""{"b":2,"a":1,"c":3}""", integer_sorting) == """{
   "a": 1,
   "b": 2,
   "c": 3
 }"""
-    )
 
-    assert (
-        pretty_string("""{"b":2,"a":1,"c":3,"z":-1}""", integer_sorting)
-        == """{
+    assert pretty_string("""{"b":2,"a":1,"c":3,"z":-1}""", integer_sorting) == """{
   "z": -1,
   "a": 1,
   "b": 2,
   "c": 3
 }"""
-    )
 
     specific_sorting = {
         None: (
@@ -349,38 +314,26 @@ def test_pretty_sorting_simple_top_level():
 
     assert pretty_string("""{}""", specific_sorting) == """{}"""
 
-    assert (
-        pretty_string("""{"a":1}""", specific_sorting)
-        == """{
+    assert pretty_string("""{"a":1}""", specific_sorting) == """{
   "a": 1
 }"""
-    )
 
-    assert (
-        pretty_string("""{"b":2,"a":1}""", specific_sorting)
-        == """{
+    assert pretty_string("""{"b":2,"a":1}""", specific_sorting) == """{
   "b": 2,
   "a": 1
 }"""
-    )
 
-    assert (
-        pretty_string("""{"b":2,"a":1,"c":3}""", specific_sorting)
-        == """{
+    assert pretty_string("""{"b":2,"a":1,"c":3}""", specific_sorting) == """{
   "b": 2,
   "a": 1,
   "c": 3
 }"""
-    )
-    assert (
-        pretty_string("""{"b":2,"a":1,"c":3,"z":-1}""", specific_sorting)
-        == """{
+    assert pretty_string("""{"b":2,"a":1,"c":3,"z":-1}""", specific_sorting) == """{
   "z": -1,
   "b": 2,
   "a": 1,
   "c": 3
 }"""
-    )
 
 
 def test_pretty_sorting_array():