Repository: beeware/briefcase
Pull request #617 (2021-09-05 23:55:55)

Added reserved python keywords
Adding a reserved python keyword list. 
## PR Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
- [ ] All new features have been tested
- [ ] All new features have been documented
- [ ] I have read the **CONTRIBUTING.md** file
- [ ] I will abide by the code of conduct



Patch:
diff --git a/changes/617.feature.rst b/changes/617.feature.rst
new file mode 100644
index 00000000..88b5b5e4
--- /dev/null
+++ b/changes/617.feature.rst
@@ -0,0 +1,1 @@
+Names that are reserved words in Python (or other common programming languages) are now prevented when creating apps.
diff --git a/docs/requirements_docs.txt b/docs/requirements_docs.txt
index cfd097bb..7059ac24 100644
--- a/docs/requirements_docs.txt
+++ b/docs/requirements_docs.txt
@@ -1,4 +1,4 @@
-sphinx<4
+sphinx
 sphinxcontrib-spelling
 pyenchant
 sphinx-autobuild
diff --git a/src/briefcase/commands/new.py b/src/briefcase/commands/new.py
index 335579b6..5d984b77 100644
--- a/src/briefcase/commands/new.py
+++ b/src/briefcase/commands/new.py
@@ -7,7 +7,7 @@ from urllib.parse import urlparse
 
 from cookiecutter import exceptions as cookiecutter_exceptions
 
-from briefcase.config import PEP508_NAME_RE
+from briefcase.config import is_valid_app_name
 from briefcase.exceptions import NetworkFailure
 
 from .base import BaseCommand, BriefcaseCommandError
@@ -103,16 +103,19 @@ class NewCommand(BaseCommand):
         :returns: True. If there are any validation problems, raises ValueError
             with a diagnostic message.
         """
-        if not PEP508_NAME_RE.match(candidate):
+        if not is_valid_app_name(candidate):
             raise ValueError(
-                "App name may only contain letters, numbers, hypens and "
-                "underscores, and may not start with a hyphen or underscore."
+                f"{candidate!r} is not a valid app name.\n\n"
+                "App names must not be reserved keywords such as 'and', 'for' and 'while'.\n"
+                "They must also be PEP508 compliant (i.e., they can only include letters,\n"
+                "numbers, '-' and '_'; must start with a letter; and cannot end with '-' or '_')."
             )
+
         if (self.base_path / candidate).exists():
             raise ValueError(
-                "A '{candidate}' directory already exists. Select a different "
+                f"A '{candidate!r}' directory already exists. Select a different "
                 "name, move to a different parent directory, or delete the "
-                "existing folder.".format(candidate=candidate)
+                "existing folder."
             )
 
         return True
diff --git a/src/briefcase/config.py b/src/briefcase/config.py
index 3eb850c4..5fb26f9e 100644
--- a/src/briefcase/config.py
+++ b/src/briefcase/config.py
@@ -1,4 +1,5 @@
 import copy
+import keyword
 import re
 from types import SimpleNamespace
 
@@ -11,14 +12,154 @@ from briefcase.platforms import get_output_formats, get_platforms
 
 from .exceptions import BriefcaseConfigError
 
-# The restriction on application naming comes from PEP508
+# PEP508 provides a basic restriction on naming
 PEP508_NAME_RE = re.compile(
     r'^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$',
     re.IGNORECASE
 )
 
-# This is the canonical definition from PEP440, modified to include
-# named groups
+# Javascript reserved keywords:
+# https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar#reserved_keywords_as_of_ecmascript_2015
+JAVASCRIPT_RESERVED_WORDS = {
+    'break',
+    'case',
+    'catch',
+    'class',
+    'const',
+    'continue',
+    'debugger',
+    'default',
+    'delete',
+    'do',
+    'else',
+    'export',
+    'extends',
+    'finally',
+    'for',
+    'function',
+    'if',
+    'import',
+    'in',
+    'instanceof',
+    'new',
+    'return',
+    'super',
+    'switch',
+    'this',
+    'throw',
+    'try',
+    'typeof',
+    'var',
+    'void',
+    'while',
+    'with',
+    'yield',
+}
+
+# Java reserved keywords
+# https://en.wikipedia.org/wiki/List_of_Java_keywords
+JAVA_RESERVED_WORDS = {
+    # Keywords
+    'abstract',
+    'assert',
+    'boolean',
+    'break',
+    'byte',
+    'case',
+    'catch',
+    'char',
+    'class',
+    'const',
+    'continue',
+    'default',
+    'do',
+    'double',
+    'else',
+    'enum',
+    'extends',
+    'final',
+    'finally',
+    'float',
+    'for',
+    'goto',
+    'if',
+    'implements',
+    'import',
+    'instanceof',
+    'int',
+    'interface',
+    'long',
+    'native',
+    'new',
+    'package',
+    'private',
+    'protected',
+    'public',
+    'return',
+    'short',
+    'static',
+    'super',
+    'switch',
+    'synchronized',
+    'this',
+    'throw',
+    'throws',
+    'transient',
+    'try',
+    'void',
+    'volatile',
+    'while',
+
+    # Reserved Identifiers
+    'exports',
+    'module',
+    'non-sealed',
+    'open',
+    'opens',
+    'permits',
+    'provides',
+    'record',
+    'requires',
+    'sealed',
+    'to',
+    'transitive',
+    'uses',
+    'var',
+    'with',
+    'yield',
+
+    # Reserved Literals
+    'true',
+    'false',
+    'null',
+
+    # Unused, but reserved.
+    'strictfp',
+}
+
+NON_PYTHON_RESERVED_WORDS = set.union(
+    JAVASCRIPT_RESERVED_WORDS,
+    JAVA_RESERVED_WORDS,
+)
+
+
+def is_valid_pep508_name(app_name):
+    "Determine if the name is valid by PEP508 rules"
+    return PEP508_NAME_RE.match(app_name)
+
+
+def is_reserved_keyword(app_name):
+    "Determine if the name is a reserved keyword"
+    return keyword.iskeyword(app_name.lower()) or app_name.lower() in NON_PYTHON_RESERVED_WORDS
+
+
+def is_valid_app_name(app_name):
+    if not is_reserved_keyword(app_name) and is_valid_pep508_name(app_name):
+        return True
+    return False
+
+
+# This is the canonical definition from PEP440, modified to include named groups
 PEP440_CANONICAL_VERSION_PATTERN_RE = re.compile(
     r'^((?P<epoch>[1-9][0-9]*)!)?'
     r'(?P<release>(0|[1-9][0-9]*)(\.(0|[1-9][0-9]*))*)'
@@ -146,44 +287,39 @@ class AppConfig(BaseConfig):
         self.template_branch = template_branch
         self.supported = supported
 
-        # Validate that the app name is valid.
-        if not PEP508_NAME_RE.match(self.app_name):
+        if not is_valid_app_name(self.app_name):
             raise BriefcaseConfigError(
-                "{self.app_name!r} is not a valid app name.\n\n"
-                "App names must be PEP508 compliant (i.e., they can only "
-                "include letters, numbers, '-' and '_'; must start with a "
-                "letter; and cannot end with '-' or '_'.".format(self=self)
+                f"{self.app_name!r} is not a valid app name.\n\n"
+                "App names must not be reserved keywords such as 'and', 'for' and 'while'.\n"
+                "They must also be PEP508 compliant (i.e., they can only include letters,\n"
+                "numbers, '-' and '_'; must start with a letter; and cannot end with '-' or '_')."
             )
 
         # Version number is PEP440 compliant:
         if not is_pep440_canonical_version(self.version):
             raise BriefcaseConfigError(
-                "Version number for {self.app_name} ({self.version}) is not valid.\n\n"
+                f"Version number for {self.app_name!r} ({self.version}) is not valid.\n\n"
                 "Version numbers must be PEP440 compliant; "
-                "see https://www.python.org/dev/peps/pep-0440/ for details.".format(
-                    self=self
-                )
+                "see https://www.python.org/dev/peps/pep-0440/ for details."
             )
 
         # Sources list doesn't include any duplicates
         source_modules = {source.rsplit('/', 1)[-1] for source in self.sources}
         if len(self.sources) != len(source_modules):
             raise BriefcaseConfigError(
-                "The `sources` list for {self.app_name} contains duplicated "
-                "package names.".format(self=self)
+                f"The `sources` list for {self.app_name!r} contains duplicated "
+                "package names."
             )
 
         # There is, at least, a source for the app module
         if self.module_name not in source_modules:
             raise BriefcaseConfigError(
-                "The `sources` list for {self.app_name} does not include a "
-                "package named '{self.module_name}'.".format(self=self)
+                f"The `sources` list for {self.app_name!r} does not include a "
+                f"package named {self.module_name!r}."
             )
 
     def __repr__(self):
-        return "<{self.bundle}.{self.app_name} v{self.version} AppConfig>".format(
-            self=self,
-        )
+        return f"<{self.bundle}.{self.app_name} v{self.version} AppConfig>"
 
     @property
     def module_name(self):


Test patch:
diff --git a/tests/commands/new/test_validate_app_name.py b/tests/commands/new/test_validate_app_name.py
index fa817ab3..211d5fdd 100644
--- a/tests/commands/new/test_validate_app_name.py
+++ b/tests/commands/new/test_validate_app_name.py
@@ -20,11 +20,25 @@ def test_valid_app_name(new_command, name):
 @pytest.mark.parametrize(
     'name',
     [
-        'hello world',  # space
-        'helloworld!',  # punctuation
+        'hello world',  # contains a space
+        'helloworld!',  # contains punctuation
         '_helloworld',  # leading underscore
         '-helloworld',  # leading hyphen
-        'existing',  # pre-existing directory
+
+        'pass',  # python keyword
+        'Pass',  # python keyword, but different case usage
+        'PASS',  # python keyword, but all upper case
+        'in',  # javascript keyword
+        'In',  # javascript keyword, but different case usage
+        'IN',  # javascript keyword, but all upper case
+        'synchronized',  # Java keyword
+        'Synchronized',  # Java keyword, but different case usage
+        'SYNCHRONIZED',  # Java keyword, but all upper case
+        'false',  # Python, Java and Javascript keyword (in different cases)
+        'False',  # Python, Java and Javascript keyword (in different cases)
+        'FALSE',  # Python, Java and Javascript keyword (in different cases)
+
+        'existing',  # pre-existing directory name
     ]
 )
 def test_invalid_app_name(new_command, name, tmp_path):
diff --git a/tests/config/test_AppConfig.py b/tests/config/test_AppConfig.py
index 95fe6050..4e7e8f6e 100644
--- a/tests/config/test_AppConfig.py
+++ b/tests/config/test_AppConfig.py
@@ -147,7 +147,10 @@ def test_valid_app_version():
 
 
 def test_invalid_app_version():
-    with pytest.raises(BriefcaseConfigError, match=r"Version number for myapp.*is not valid\."):
+    with pytest.raises(
+        BriefcaseConfigError,
+        match=r"Version number for 'myapp' \(foobar\) is not valid\."
+    ):
         AppConfig(
             app_name="myapp",
             version="foobar",
diff --git a/tests/config/test_is_reserved_keyword.py b/tests/config/test_is_reserved_keyword.py
new file mode 100644
index 00000000..a9f19f98
--- /dev/null
+++ b/tests/config/test_is_reserved_keyword.py
@@ -0,0 +1,47 @@
+import pytest
+
+from briefcase.config import is_reserved_keyword
+
+
+@pytest.mark.parametrize(
+    'name',
+    [
+        # These names may not be valid identifiers,
+        # but they're not reserved words in any language we care about.
+        'BLACK',
+        'purple',
+        '9pink',
+        'Yellow',
+        'green',
+        'hello world',
+    ]
+)
+def test_is_not_reserved_keyword_violation(name):
+    "Test that names not in the keywords list return false."
+    assert not is_reserved_keyword(name)
+
+
+@pytest.mark.parametrize(
+    'name',
+    [
+        # Python specific keyword, in various case variants
+        'pass',
+        'Pass',
+        'PASS',
+        # Javascript specific keyword, in various case variants
+        'in',
+        'In',
+        'IN',
+        # Java specific keyword, in various case variants
+        'synchronized',
+        'Synchronized',
+        'SYNCHRONIZED',
+        # Keyword in multiple languages, in various case variants
+        'false',
+        'False',
+        'FALSE',
+    ]
+)
+def test_is_reserved_keyword(name):
+    "Test that names in the reserved_keywords list returns true"
+    assert is_reserved_keyword(name)
diff --git a/tests/config/test_is_valid_app_name.py b/tests/config/test_is_valid_app_name.py
new file mode 100644
index 00000000..7be36381
--- /dev/null
+++ b/tests/config/test_is_valid_app_name.py
@@ -0,0 +1,38 @@
+import pytest
+
+from briefcase.config import is_valid_app_name
+
+
+@pytest.mark.parametrize(
+    'name',
+    [
+        'helloworld',
+        'helloWorld',
+        'hello42world',
+        '42helloworld',
+        'hello_world',
+        'hello-world',
+    ]
+)
+def test_is_valid_app_name(name):
+    "Test that valid app names are accepted"
+    assert is_valid_app_name(name)
+
+
+@pytest.mark.parametrize(
+    'name',
+    [
+        'hello world',
+        'helloworld!',
+        '_helloworld',
+        '-helloworld',
+        'switch',
+        'pass',
+        'false',
+        'False',
+        'YIELD',
+    ]
+)
+def test_is_invalid_app_name(name):
+    "Test that invalid app names are rejected"
+    assert not is_valid_app_name(name)