Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make internal prefixes available to linter rules #26188

Merged
merged 2 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
51 changes: 39 additions & 12 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=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=internal_prefixes),
)


Expand All @@ -147,8 +159,11 @@ 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=internal_prefixes
) or check_pascal_case(
context, node, internal_prefixes=internal_prefixes
jabraham17 marked this conversation as resolved.
Show resolved Hide resolved
)

@driver.basic_rule(Record)
Expand All @@ -157,7 +172,10 @@ 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=internal_prefixes
)

@driver.basic_rule(Function)
def CamelCaseFunctions(context: Context, node: Function):
Expand All @@ -177,23 +195,32 @@ 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=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=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=internal_prefixes
)

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