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

Move MetaWarning classes into new cli module #142

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
98 changes: 2 additions & 96 deletions codebasin/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
import argparse
import logging
import os
import re
import sys

from codebasin import CodeBase, config, finder, report, util
from codebasin.cli import Formatter, WarningAggregator

log = logging.getLogger("codebasin")
version = "1.2.0"
Expand Down Expand Up @@ -54,100 +54,6 @@ def _help_string(*lines: str, is_long=False, is_last=False):
return result


class Formatter(logging.Formatter):
def __init__(self, *, colors: bool = False):
self.colors = colors

def format(self, record: logging.LogRecord) -> str:
msg = record.msg
level = record.levelname.lower()

# Display info messages with no special formatting.
if level == "info":
return f"{msg}"

# Drop colors if requested.
if not self.colors:
return f"{level}: {msg}"

# Otherwise, use ASCII codes to improve readability.
BOLD = "\033[1m"
DEFAULT = "\033[39m"
YELLOW = "\033[93m"
RED = "\033[91m"
RESET = "\033[0m"

if level == "warning":
color = YELLOW
elif level == "error":
color = RED
else:
color = DEFAULT
return f"{BOLD}{color}{level}{RESET}: {msg}"


class MetaWarning:
"""
A MetaWarning is used to represent multiple warnings, and provide suggested
actions to the user.
"""

def __init__(self, regex: str, msg: str):
self.regex = re.compile(regex)
self.msg = msg
self._count = 0

def inspect(self, record: logging.LogRecord):
if self.regex.search(record.msg):
self._count += 1

def warn(self):
if self._count == 0:
return
log.warning(self.msg.format(self._count))


class WarningAggregator(logging.Filter):
"""
Inspect warnings to generate meta-warnings and statistics.
"""

def __init__(self):
self.meta_warnings = [
MetaWarning(".", "{} warnings generated during preprocessing."),
MetaWarning(
"user include",
"{} user include files could not be found.\n"
+ " These could contain important macros and includes.\n"
+ " Suggested solutions:\n"
+ " - Check that the file(s) exist in the code base.\n"
+ " - Check the include paths in the compilation database.\n"
+ " - Check if the include(s) should have used '<>'.",
),
MetaWarning(
"system include",
"{} system include files could not be found.\n"
+ " These could define important feature macros.\n"
+ " Suggested solutions:\n"
+ " - Check that the file(s) exist on your system.\n"
+ " - Use .cbi/config to define system include paths.\n"
+ " - Use .cbi/config to define important macros.",
),
]

def filter(self, record: logging.LogRecord) -> bool:
if record.levelno == logging.WARNING:
for meta_warning in self.meta_warnings:
meta_warning.inspect(record)

# Do not filter anything.
return True

def warn(self):
for meta_warning in self.meta_warnings:
meta_warning.warn()


def _main():
# Read command-line arguments
parser = argparse.ArgumentParser(
Expand Down Expand Up @@ -325,7 +231,7 @@ def _main():
# Generate meta-warnings and statistics.
# Temporarily override log_level to ensure they are visible.
stdout_handler.setLevel(logging.WARNING)
aggregator.warn()
aggregator.warn(log)
stdout_handler.setLevel(log_level)

# Count lines for platforms
Expand Down
185 changes: 185 additions & 0 deletions codebasin/cli.py
Copy link
Contributor

@laserkelvin laserkelvin Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it's just me, but the naming of this module doesn't seem to really fit its contents?

It's called cli.py but it contains logging stuff, and if we're moving things around, would it make sense to name it something that maps better? (log_utils.py?)

Otherwise, refactors and new tests look good to me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I can see the confusion. I'll explain my logic, and you can tell me if it makes sense 😆.

The thinking behind this "cli" module was that it would contain things that were only useful for the user-facing command-line interface, but which might be shared across commands (e.g., the Formatter is currently used by both codebasin and cbicov).

I was basically looking for a way to separate things that we use to build the user-facing CLI from the modules and interfaces that we (eventually) want people to be able to use when importing the codebasin package into another script.

Maybe it would be better to have this as something like a cli package, containing a logging.py? Or an _internal package containing a logging.py? I'm definitely open to suggestions here.

Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
#!/usr/bin/env python3
# Copyright (C) 2019-2024 Intel Corporation
# SPDX-License-Identifier: BSD-3-Clause

import logging
import re


class Formatter(logging.Formatter):
"""
A Formatter that formats LogRecords using a format inspired by compilers
like gcc/clang, with optional colors.
"""

def __init__(self, *, colors: bool = False):
"""
Initialize this Formatter.

Parameters
----------
colors: bool, default: False
Whether to colorize the output.
"""
self.colors = colors

def format(self, record: logging.LogRecord) -> str:
"""
Format the specified record.

Parameters
----------
record: logging.LogRecord
The record to format.

Returns
-------
str
The formatted output string.
"""
msg = record.msg
level = record.levelname.lower()

# Display info messages with no special formatting.
if level == "info":
return f"{msg}"

# Drop colors if requested.
if not self.colors:
return f"{level}: {msg}"

# Otherwise, use ASCII codes to improve readability.
BOLD = "\033[1m"
DEFAULT = "\033[39m"
YELLOW = "\033[93m"
RED = "\033[91m"
RESET = "\033[0m"

if level == "warning":
color = YELLOW
elif level == "error":
color = RED
else:
color = DEFAULT
return f"{BOLD}{color}{level}{RESET}: {msg}"


class MetaWarning:
"""
A MetaWarning is used to represent multiple warnings, and to provide
suggested actions to the user.
"""

def __init__(self, regex: str, msg: str):
"""
Initialize a new MetaWarning.

Parameters
----------
regex: str
A regular expression used to identify constituent warnings.
If any warning matches `regex`, this MetaWarning will trigger.

msg: str
The message to display when this MetaWarning is triggered.
"""
self.regex = re.compile(regex)
self.msg = msg
self._count = 0

def inspect(self, record: logging.LogRecord) -> bool:
"""
Inspect a LogRecord to determine if it matches this MetaWarning.

Parameters
----------
record: logging.LogRecord
The LogRecord to inspect.

Returns
-------
bool
True if `record` matches this MetaWarning and False otherwise.
"""
if self.regex.search(record.msg):
self._count += 1
return True
return False

def warn(self, logger: logging.Logger):
"""
Produce the warning associated with this MetaWarning.

Parameters
----------
log: logging.Logger
The Logger that should be used to generate the MetaWarning.
"""
if self._count == 0:
return
logger.warning(self.msg.format(self._count))


class WarningAggregator(logging.Filter):
"""
A WarningAggregator is a logging.Filter that inspects warnings to generate
meta-warnings and statistics. It does not perform any filtering, but uses
the logging.Filter mechanism as a hook to automatically inspect every
warning passing through a logger.
"""

def __init__(self):
self.meta_warnings = [
MetaWarning(".", "{} warnings generated during preprocessing."),
MetaWarning(
"user include",
"{} user include files could not be found.\n"
+ " These could contain important macros and includes.\n"
+ " Suggested solutions:\n"
+ " - Check that the file(s) exist in the code base.\n"
+ " - Check the include paths in the compilation database.\n"
+ " - Check if the include(s) should have used '<>'.",
),
MetaWarning(
"system include",
"{} system include files could not be found.\n"
+ " These could define important feature macros.\n"
+ " Suggested solutions:\n"
+ " - Check that the file(s) exist on your system.\n"
+ " - Use .cbi/config to define system include paths.\n"
+ " - Use .cbi/config to define important macros.",
),
]

def filter(self, record: logging.LogRecord) -> bool:
"""
Inspect the specified LogRecord, attempting to match it against each
possible MetaWarning.

Parameters
----------
record: logging.LogRecord
The record to inspect.

Returns
-------
bool
True, to prevent any warnings from being filtered.
"""
if record.levelno == logging.WARNING:
for meta_warning in self.meta_warnings:
meta_warning.inspect(record)
return True

def warn(self, logger: logging.Logger):
"""
Produce the warning associated with any MetaWarning(s) that were
matched by this WarningAggregator.

Parameters
----------
logger: logging.Logger
The Logger that should be used to generate the MetaWarning(s).
"""
for meta_warning in self.meta_warnings:
meta_warning.warn(logger)
Empty file added tests/cli/__init__.py
Empty file.
55 changes: 55 additions & 0 deletions tests/cli/test_formatter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# Copyright (C) 2019-2024 Intel Corporation
# SPDX-License-Identifier: BSD-3-Clause

import logging
import unittest

from codebasin.cli import Formatter


class TestFormatter(unittest.TestCase):
"""
Test Formatter class.
"""

def setUp(self):
logging.disable()

def test_constructor(self):
"""Check constructor arguments"""
self.assertTrue(Formatter(colors=True).colors)
self.assertFalse(Formatter(colors=False).colors)
self.assertFalse(Formatter().colors)

def test_format(self):
"""Check output format"""
levels = ["DEBUG", "INFO", "WARNING", "ERROR"]
colors = ["\033[39m", "\033[39m", "\033[93m", "\033[91m"]
for colorize in [True, False]:
for levelname, color in zip(levels, colors):
formatter = Formatter(colors=colorize)
with self.subTest(
colorize=colorize,
levelname=levelname,
color=color,
):
record = logging.makeLogRecord(
{
"msg": "Testing",
"levelname": levelname,
},
)
msg = record.msg
level = record.levelname.lower()
output = formatter.format(record)
if level == "info":
expected = msg
elif colorize:
expected = f"\033[1m{color}{level}\033[0m: {msg}"
else:
expected = f"{level}: {msg}"
self.assertEqual(output, expected)


if __name__ == "__main__":
unittest.main()
Loading
Loading