Skip to content

Commit

Permalink
Make internal prefixes available to linter rules (#26188)
Browse files Browse the repository at this point in the history
Makes the internal prefixes (as specified by `--internal-prefix chpl_`)
be exposed to linter rules, rather than just the driver. Technically
this was already possible, this PR just makes the naming based rules
exclude the prefixes, such that `chpl_camelCase` is considered to be
camelCase

[Reviewed by @DanilaFe]
  • Loading branch information
jabraham17 authored Nov 4, 2024
2 parents 7a8df7f + 6562f23 commit 294df09
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 15 deletions.
9 changes: 9 additions & 0 deletions tools/chplcheck/src/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class Config:
enabled_rules: List[str]
skip_unstable: bool
internal_prefixes: List[str]
check_internal_prefixes: bool
add_rules: List[str]

@classmethod
Expand Down Expand Up @@ -64,6 +65,13 @@ def add_arguments(cls, parser: argparse.ArgumentParser, prefix: str = ""):
default=[],
help="Add a prefix to the list of internal prefixes used when linting",
)
parser.add_argument(
f"--{prefix}check-internal-prefix",
action="append",
dest="chplcheck_check_internal_prefixes",
default=False,
help="Check symbols with internal prefixes when linting",
)
parser.add_argument(
f"--{prefix}add-rules",
action="append",
Expand All @@ -80,5 +88,6 @@ def from_args(cls, args: Union[argparse.Namespace, Dict[str, Any]]):
enabled_rules=args["chplcheck_enabled_rules"],
skip_unstable=args["chplcheck_skip_unstable"],
internal_prefixes=args["chplcheck_internal_prefixes"],
check_internal_prefixes=args["chplcheck_check_internal_prefixes"],
add_rules=args["chplcheck_add_rules"],
)
10 changes: 8 additions & 2 deletions tools/chplcheck/src/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,10 @@ def should_check_rule(

return True

def _has_internal_name(self, node: chapel.AstNode):
def has_internal_prefix(self, node: chapel.AstNode) -> bool:
"""
Check if a node has a name that starts with one of the internal prefixes.
"""
if not hasattr(node, "name"):
return False
return any(
Expand Down Expand Up @@ -256,7 +259,10 @@ def run_checks(
for ast in asts:
for rule in itertools.chain(self.BasicRules, self.AdvancedRules):
for toreport in self._check_rule(context, ast, rule):
if self._has_internal_name(toreport[0]):
if (
not self.config.check_internal_prefixes
and self.has_internal_prefix(toreport[0])
):
continue

yield toreport
45 changes: 32 additions & 13 deletions tools/chplcheck/src/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,26 +113,38 @@ def fixit_remove_unused_node(
return None


def name_for_linting(context: Context, node: NamedDecl) -> str:
def name_for_linting(
context: Context, node: NamedDecl, internal_prefixes: List[str] = []
) -> str:
name = node.name()

# Strip dollar signs.
name = name.replace("$", "")

# TODO: thread `internal_prefixes` through to this and strip them for linting
# Strip internal prefixes.
for p in internal_prefixes:
if name.startswith(p):
name = name[len(p) :]
break

return name


def check_camel_case(context: Context, node: NamedDecl):
def check_camel_case(
context: Context, node: NamedDecl, internal_prefixes: List[str] = []
):
return re.fullmatch(
r"([a-z]+([A-Z][a-z]*|\d+)*|[A-Z]+)?", name_for_linting(context, node)
r"([a-z]+([A-Z][a-z]*|\d+)*|[A-Z]+)?",
name_for_linting(context, node, internal_prefixes),
)


def check_pascal_case(context: Context, node: NamedDecl):
def check_pascal_case(
context: Context, node: NamedDecl, internal_prefixes: List[str] = []
):
return re.fullmatch(
r"(([A-Z][a-z]*|\d+)+|[A-Z]+)?", name_for_linting(context, node)
r"(([A-Z][a-z]*|\d+)+|[A-Z]+)?",
name_for_linting(context, node, internal_prefixes),
)


Expand All @@ -147,17 +159,19 @@ def CamelOrPascalCaseVariables(context: Context, node: VarLikeDecl):
return True
if node.linkage() == "extern":
return True
return check_camel_case(context, node) or check_pascal_case(
context, node
)
internal_prefixes = driver.config.internal_prefixes
return check_camel_case(
context, node, internal_prefixes
) or check_pascal_case(context, node, internal_prefixes)

@driver.basic_rule(Record)
def CamelCaseRecords(context: Context, node: Record):
"""
Warn for records that are not 'camelCase'.
"""

return check_camel_case(context, node)
internal_prefixes = driver.config.internal_prefixes
return check_camel_case(context, node, internal_prefixes)

@driver.basic_rule(Function)
def CamelCaseFunctions(context: Context, node: Function):
Expand All @@ -177,23 +191,28 @@ def CamelCaseFunctions(context: Context, node: Function):
if node.name() == "init=":
return True

return check_camel_case(context, node)
internal_prefixes = driver.config.internal_prefixes
return check_camel_case(context, node, internal_prefixes)

@driver.basic_rule(Class)
def PascalCaseClasses(context: Context, node: Class):
"""
Warn for classes that are not 'PascalCase'.
"""

return check_pascal_case(context, node)
internal_prefixes = driver.config.internal_prefixes
return check_pascal_case(context, node, internal_prefixes)

@driver.basic_rule(Module)
def PascalCaseModules(context: Context, node: Module):
"""
Warn for modules that are not 'PascalCase'.
"""

return node.kind() == "implicit" or check_pascal_case(context, node)
internal_prefixes = driver.config.internal_prefixes
return node.kind() == "implicit" or check_pascal_case(
context, node, internal_prefixes
)

@driver.basic_rule(Module, default=False)
def UseExplicitModules(_, node: Module):
Expand Down

0 comments on commit 294df09

Please sign in to comment.