Add functionality to validate analyzer and checker options

[noraz31]

When specifying invalid checker and analyzer config, the CodeChecker analyze command used to give a warning but continued running.
This functionality throws an error and prevents the analysis from running in this case.
CODECHKR-5784

[Szelethus]

Please check the analyzer name as well, and clear up the wording in the error msg.

[Szelethus]

COuld decrease indentation here as well with an early return.

[Szelethus]

Wording is off here: under analyzer, we mean the actual plugins that CodeChecker implements around a static analyzer tool: clangsa, clang-tidy, cppcheck and gcc. While we should definitely check if the user gave a wrong analyzer name, but at this point you are checking the name of the config, not the analyzer. Your message states

$ CodeChecker check -b "clang++ -c test.cpp -o results/prune_paths_full -c --analyzer-config clangsa:hahaha=false --analyzers=clangsa
[ERROR 2024-04-09 11:21] - Invalid --analyzer-config parameter: clangsa has no analyzer named hahaha. Use the 'CodeChecker analyzers --analyzer-config clangsa' command to check available analyzers.

But should instead state:

$ CodeChecker check -b "clang++ -c test.cpp -o results/prune_paths_full -c --analyzer-config clangsa:hahaha=false --analyzers=clangsa
[ERROR 2024-04-09 11:21] - Invalid --analyzer-config parameter: clangsa has no config named hahaha. Use the 'CodeChecker analyzers --analyzer-config clangsa' command to check available configs.

[Szelethus]

Please don't take args as a parmeter.

"args", which contains everything you need to get CodeChecker going (user invocation, default values, etc). If you want to create a unit test for this function, it would look something like this:

args = {'analyzer_config' : ['clangsa:HAHAHA=true']}
self.assertFalse(validate_analyzer_parameter(args, 'clangsa'))

It is not impossible to conjure up an args object to pass to this function for testing, but its fragile: if we ever need to read more values from args after a new feature or refactoring, all tests needs changing as well. Consider the following alterative:

# analyzer/tests/unit/test_checker_handling.py
self.assertFalse(validate_analyzer_parameter(["clangsa:HAHAHA=true"]))

This philosophy is one of the reasons behind that have soooo many function tests instead of unit tests. I may be painting a greater evil than what is actually present in the patch, but the idea stands.

Prefer having an easy-to-test boolean function

On another issue, its fine if you have a non-returning validator function, but it'd be nice for testing purposes and clarify if you had a boolean function as well for both clarity and testing pusposes.

I'd imagine the main function to look like this (which would be more in line with the existing input testing):

for analyzer_config in args.analyzer_configs:
  if not is_valid_analyzer_config(analyzer_config):
    LOG.error("SORT YOUR INPUT OUT!")
    sys.exit(1);
for checker_config in args.checker_configs:
  if not is_valid_checker_config(checker_config):
    LOG.error("SORT YOUR INPUT OUT!")
    sys.exit(1);

And everything else should happen behind the scenes.

[Szelethus]

Analyzer value checking is still a little off. My other comments are just nitpicking.

[Szelethus]

Why didn't you use the bracket ([]) operator? Also, you check all analyzer configs here, but only check the very first analyzer config for a valid analyzer field?

For this example, the second analyzer config this has an invalid analyzer field, but you miss it:

CodeChecker check -b "g++ test.cpp" --analyzer-config clangsa:prune-paths=false --analyzer-config:OOGABOOGA:no-false-positives=true

[Szelethus]

Almost there!

[Szelethus]

Fun fact:

def func(a, b, c = 5): <--- these are parameters
  pass

func("a", 3) <--- these are arguments

Suggested change

	LOG.error(f"Invalid --analyzer-config parameter: {analyzer} is"
	f" not a supported analyzer. The supported analyzers are"
	f" {', '.join(a for a in supported_analyzers)}.")
	LOG.error(f"Invalid argument to --analyzer-config: {analyzer} "
	f"is not a supported analyzer. Supported analyzers are :"
	f" {', '.join(a for a in supported_analyzers)}.")

[Szelethus]

Same here.

[Szelethus]

If the validation failed and this suppression flag isn't present, try to advertise it, somewhat like what I've done in #3866:

Suggested change

	sys.exit(1)
	LOG.info("Although it is not reccomended, if you want to "
	"suppress errors relating to unknown "
	"analyzer/checker configs, consider using the option "
	"'--no-missing-checker-error'")
	sys.exit(1)

[Szelethus]

Also, please create tests for the interaction of the suppression flag and this validation checking.

[Szelethus]

LGTM, very well done!