Skip to content

Commit

Permalink
First part of review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
vodorok committed Aug 19, 2022
1 parent 258833e commit 5ec21d7
Show file tree
Hide file tree
Showing 18 changed files with 104 additions and 94 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/pypi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ jobs:
shell: powershell
run: |
choco install llvm;
choco install cppcheck;
choco install --ignore-package-exit-codes cppcheck;
echo "C:\Program Files\LLVM\bin" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append
- name: "Install pypi package"
Expand Down
2 changes: 1 addition & 1 deletion analyzer/codechecker_analyzer/analysis_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ def signal_handler(signum, frame):
if not os.path.exists(reproducer_dir) and generate_reproducer:
os.makedirs(reproducer_dir)

# Cppcheck raw output directory
# Cppcheck raw output directory.
cppcheck_dir = os.path.join(output_path, "cppcheck")
if not os.path.exists(cppcheck_dir):
os.makedirs(cppcheck_dir)
Expand Down
4 changes: 3 additions & 1 deletion analyzer/codechecker_analyzer/analyzers/analyzer_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ def resolve_missing_binary(cls, configured_binary, environ):
@classmethod
def version_compatible(cls, configured_binary, environ):
"""
Checker the version compatibility of the given analyzer binary.
CodeChecker can only execute certain versions of analyzers.
This function should return True if the analyzer binary is
compatible with the current CodeChecker version.
"""
raise NotImplementedError("Subclasses should implement this!")

Expand Down
2 changes: 1 addition & 1 deletion analyzer/codechecker_analyzer/analyzers/analyzer_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ def check_supported_analyzers(analyzers, context):
available_analyzer = False

if not analyzer_bin or \
not host_check.check_clang(analyzer_bin, check_env):
not host_check.check_analyzer(analyzer_bin, check_env):
# Analyzers unavailable under absolute paths are deliberately a
# configuration problem.
failed_analyzers.add((analyzer_name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ def resolve_missing_binary(cls, configured_binary, environ):
@classmethod
def version_compatible(cls, configured_binary, environ):
"""
Checker the version compatibility of the given analyzer binary.
Check the version compatibility of the given analyzer binary.
"""
return True

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ def resolve_missing_binary(cls, configured_binary, environ):
@classmethod
def version_compatible(cls, configured_binary, environ):
"""
Checker the version compatibility of the given analyzer binary.
Check the version compatibility of the given analyzer binary.
"""
return True

Expand Down
67 changes: 37 additions & 30 deletions analyzer/codechecker_analyzer/analyzers/cppcheck/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
# -------------------------------------------------------------------------
"""
"""
# TODO distutils will be removed in python3.12
from distutils.version import StrictVersion
from pathlib import Path
import os
Expand All @@ -32,7 +33,8 @@

def parse_checkers(cppcheck_output):
"""
Parse cppcheck checkers list.
Parse cppcheck checkers list given by '--errorlist' flag. Return a list of
(checker_name, description) pairs.
"""
checkers = []

Expand All @@ -43,7 +45,7 @@ def parse_checkers(cppcheck_output):
name = error.attrib.get('id')
if name:
name = "cppcheck-" + name
msg = error.attrib.get('msg')
msg = str(error.attrib.get('msg') or '')
# TODO: Check severity handling in cppcheck
# severity = error.attrib.get('severity')

Expand All @@ -62,18 +64,16 @@ def parse_version(cppcheck_output):
if match:
return StrictVersion(match.group('version'))

return None


class Cppcheck(analyzer_base.SourceAnalyzer):
"""
Constructs the clang tidy analyzer commands.
Constructs the Cppcheck analyzer commands.
"""

ANALYZER_NAME = 'cppcheck'

def add_checker_config(self, checker_cfg):
LOG.error("Not implemented yet")
LOG.error("Checker configuration for Cppcheck is not implemented yet")

def get_analyzer_mentioned_files(self, output):
"""
Expand All @@ -92,22 +92,17 @@ def construct_analyzer_cmd(self, result_handler):

analyzer_cmd = [config.analyzer_binary]

# Enable or disable checkers.
enabled_severity_levels = set()

# TODO implement a more granular commandline checker config
# Cppcheck runs with all checkers enabled for the time being
# the unneded checkers are passed as suppressed checkers
analyzer_cmd.append('--enable=all')

if enabled_severity_levels:
analyzer_cmd.append('--enable=' +
','.join(enabled_severity_levels))

for checker_name, value in config.checks().items():
if value[0] == CheckerState.disabled:
# TODO python3.9 removeprefix method would be nicer
# than startswith and a hardcoded slicing
if checker_name.startswith("cppcheck-"):
checker_name = checker_name[9:]
# TODO python3.9 removeprefix method is better than lstrip
analyzer_cmd.append('--suppress=' + checker_name)

# unusedFunction check is for whole program analysis,
Expand All @@ -117,17 +112,25 @@ def construct_analyzer_cmd(self, result_handler):
# Add extra arguments.
analyzer_cmd.extend(config.analyzer_extra_arguments)

# Add includes.
# Pass includes, and macro (un)definitions
for analyzer_option in self.buildaction.analyzer_options:
if analyzer_option.startswith("-I") or \
analyzer_option.startswith("-U") or \
analyzer_option.startswith("-D"):
analyzer_cmd.extend([analyzer_option])
elif analyzer_option.startswith("-i"):
analyzer_cmd.extend(["-I" + analyzer_option[2:]])
elif analyzer_option.startswith("-std"):
standard = analyzer_option.split("=")[-1] \
.lower().replace("gnu", "c")
analyzer_cmd.extend(["--std=" + standard])

# TODO fix this in a follow up patch, because it is failing
# the macos pypy test.
# analyzer_cmd.extend(["--std=" +
# self.buildaction.compiler_standard
# .split("=")[-1].lower().replace("gnu", "c")])

# By default there is no platform configuration,
# but a platform definition xml can be specified.
if 'platform' in config.analyzer_config:
Expand All @@ -146,10 +149,6 @@ def construct_analyzer_cmd(self, result_handler):
analyzer_cmd.extend(
["--library=" + str(Path(lib).absolute())])

# Cppcheck does not handle compiler includes well
# for include in self.buildaction.compiler_includes:
# analyzer_cmd.extend(['-I', include])

# TODO Suggest a better place for this
# cppcheck wont create the output folders for itself
output_dir = Path(result_handler.workspace, "cppcheck",
Expand Down Expand Up @@ -177,11 +176,13 @@ def get_analyzer_checkers(
command = [cfg_handler.analyzer_binary, "--errorlist"]

try:
command = shlex.split(' '.join(command))
result = subprocess.check_output(command, env=env)
return parse_checkers(result)
except (subprocess.CalledProcessError, OSError):
return []
except (subprocess.CalledProcessError) as e:
LOG.error(e.stderr)
except (OSError) as e:
LOG.error(e.errno)
return []

@classmethod
def get_analyzer_config(cls, cfg_handler, environ):
Expand All @@ -202,7 +203,11 @@ def get_checker_config(cls, cfg_handler, environ):

def post_analyze(self, result_handler):
"""
Copies the generated plist file with a unique name,
Post process the reuslts after the analysis.
Will copy the plist files created by cppcheck into the
root of the reports folder.
Renames the source plist files to *.plist.bak because
The report parsing of the Parse command is done recursively.
"""
# Cppcheck can generate an id into the output plist file name
Expand All @@ -217,8 +222,14 @@ def post_analyze(self, result_handler):
codechecker_out = os.path.join(result_handler.workspace,
result_handler.analyzer_result_file)

shutil.copy2(cppcheck_out, codechecker_out)
Path(cppcheck_out).rename(str(cppcheck_out) + ".bak")
# plists generated by cppcheck are still "parsable" by our plist
# parser. Renaming is needed to circumvent the processing of the
# raw files.
try:
shutil.copy2(cppcheck_out, codechecker_out)
Path(cppcheck_out).rename(str(cppcheck_out) + ".bak")
except(OSError) as e:
LOG.error(e.errno)

@classmethod
def resolve_missing_binary(cls, configured_binary, env):
Expand Down Expand Up @@ -263,7 +274,7 @@ def __get_analyzer_version(cls, analyzer_binary, env):
@classmethod
def version_compatible(cls, configured_binary, environ):
"""
Checker the version compatibility of the given analyzer binary.
Check the version compatibility of the given analyzer binary.
"""
analyzer_version = \
cls.__get_analyzer_version(configured_binary, environ)
Expand Down Expand Up @@ -315,10 +326,6 @@ def construct_config_handler(cls, args, context):
# Overwrite PATH to contain only the parent of the cppcheck binary.
if os.path.isabs(handler.analyzer_binary):
check_env['PATH'] = os.path.dirname(handler.analyzer_binary)
# cppcheck_bin = cls.resolve_missing_binary('cppcheck', check_env)

# handler.compiler_resource_dir = \
# host_check.get_resource_dir(cppcheck_bin, context)

checkers = cls.get_analyzer_checkers(handler, check_env)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ class CppcheckConfigHandler(config_handler.AnalyzerConfigHandler):
def initialize_checkers(self,
analyzer_context,
checkers,
cmdline_enable=...,
cmdline_enable=None,
enable_all=False):
if not cmdline_enable:
cmdline_enable = list()
"""
Set all the default checkers to disabled. This will ensure that
--enable=all will not run with all the possible checkers
Expand All @@ -31,9 +33,11 @@ def initialize_checkers(self,
cmdline_enable,
enable_all)

# Set all the default checkers to disabled. This will ensure that
# --enable=all will not run with all the possible checkers
# Set all the checkers with default CheckerState checkers to
# disabled. This will ensure that --enable=all will not run with
# all the possible checkers. All the checkers that are in the default
# profile (or configured othewise, eg.: from the cli) should be
# already enabled at this point.
for checker_name, data in self.checks().items():
if data[0] == CheckerState.default:
self.set_checker_enabled(checker_name, enabled=False)
return
19 changes: 17 additions & 2 deletions analyzer/codechecker_analyzer/analyzers/cppcheck/result_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
Result handler for Cppcheck.
"""
from typing import Optional

from codechecker_report_converter.report.parser.base import AnalyzerInfo
from codechecker_report_converter.analyzers.cppcheck.analyzer_result import \
AnalyzerResult
from codechecker_report_converter.report import report_file
from codechecker_report_converter.report import BugPathEvent, \
Range, report_file
from codechecker_report_converter.report.hash import get_report_hash, HashType

from codechecker_common.logger import get_logger
Expand All @@ -25,7 +27,7 @@

class CppcheckResultHandler(ResultHandler):
"""
Use context free hash if enabled.
Create analyzer result file for Cppcheck output.
"""

def __init__(self, *args, **kwargs):
Expand All @@ -43,8 +45,10 @@ def postprocess_result(self, skip_handlers: Optional[SkipListHandlers]):
reports = report_file.get_reports(
self.analyzer_result_file, self.checker_labels,
source_dir_path=self.source_dir_path)

reports = [r for r in reports if not r.skip(skip_handlers)]
for report in reports:
# TODO check if prefix cascading still occurs.
if not report.checker_name.startswith("cppcheck-"):
report.checker_name = "cppcheck-" + report.checker_name

Expand All @@ -56,6 +60,17 @@ def postprocess_result(self, skip_handlers: Optional[SkipListHandlers]):

for report in reports:
report.report_hash = get_report_hash(report, hash_type)
bpe = BugPathEvent(
report.message,
report.file,
report.line,
report.column,
Range(report.line,
report.column,
report.line,
report.column))
if bpe != report.bug_path_events[-1]:
report.bug_path_events.append(bpe)

report_file.create(
self.analyzer_result_file, reports, self.checker_labels,
Expand Down
6 changes: 4 additions & 2 deletions analyzer/codechecker_analyzer/cmd/checkers.py
Original file line number Diff line number Diff line change
Expand Up @@ -672,8 +672,10 @@ def __print_checker_config(args: argparse.Namespace):
configs = analyzer_class.get_checker_config(config_handler,
analyzer_environment)
if not configs:
analyzer_failures.append(analyzer)
continue
# Checker configurations are not supported by cppcheck
if analyzer != "cppcheck":
analyzer_failures.append(analyzer)
continue

rows.extend((':'.join((analyzer, c[0])), c[1]) if 'details' in args
else (':'.join((analyzer, c[0])),) for c in configs)
Expand Down
2 changes: 1 addition & 1 deletion analyzer/codechecker_analyzer/host_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
LOG = get_logger('analyzer')


def check_clang(compiler_bin, env):
def check_analyzer(compiler_bin, env):
"""
Simple check if clang is available.
"""
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ bool warning(int param)
int main()
{
return 0;
}
}

1 change: 0 additions & 1 deletion analyzer/tests/functional/ctu_failure/test_ctu_failure.py
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,5 @@ def __getClangSaPath(self):
json_data = json.loads(output)

for i in range(len(json_data)):
print(json_data[i]["name"])
if json_data[i]["name"] == "clangsa":
return json_data[i]["path"]
2 changes: 1 addition & 1 deletion docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ macOS (OS X) development environment.

# Main features
## Command line C/C++ Analysis
* Executes [_Clang-Tidy_](http://clang.llvm.org/extra/clang-tidy/), [_Cppcheck_](https://cppcheck.sourceforge.io/) and [_Clang Static Analyzer_](http://clang-analyzer.llvm.org/) with Cross-Translation Unit analysis, Statistical Analysis (when checkers are available).
* Executes [_Clang-Tidy_](http://clang.llvm.org/extra/clang-tidy/), [_Clang Static Analyzer_](http://clang-analyzer.llvm.org/) with Cross-Translation Unit analysis, Statistical Analysis (when checkers are available), and [_Cppcheck_](https://cppcheck.sourceforge.io/).
* Creates the JSON compilation database by wiretapping any build process (e.g., `CodeChecker log -b "make"`).
* Automatically analyzes GCC cross-compiled projects: detecting GCC or Clang compiler configuration and forming the corresponding clang analyzer invocations.
* Incremental analysis: Only the changed files and its dependencies need to be reanalyzed.
Expand Down
Loading

0 comments on commit 5ec21d7

Please sign in to comment.