[Adjust Rule] SIM116 extension for more edge cases

[reactive-firewall]

Desired change

• Rule(s): SIM116
• Adjustment: Expand rules to cover "cond in key_like" and or-chains

Explanation

Why is the adjustment necessary / better?

I'm not sure if this should be extended to new rules or just part of SIM116.

some hasty pseudo-code

I've attempted to simplify the "in" keyword use-case here (albeit in is far more nuanced than ==)

# assumptions (e.g., should be testable with ast tinkering)
key_like_match = "SIM116"
key_like_miss = "S901"
conn_match = "SIM"
conn_miss = "S"

# anti-pattern
if "SIM" in key_like_match:
    func_match()  # or pass or return const
elif "S" in key_like_miss:  # <-- unreachable for "matches" (will need order)
    func_miss()
else:
    pass  # or return const or func_default()


# translations and checks
if instanceof(key_like_match, str):
    key_like_n = key_like_match
if instanceof(key_like_miss, str):
    key_like_n_plus_1 = key_like_miss


if key_like_n and key_like_n_plus_1:

    # best-practice
    mapping = {
        key_like_n: func_match(),  # or pass or return const
        key_like_n_plus_1: func_miss(),  # or pass or return const
    }

    # idea
    for key_like in mapping:  # e.g., mapping.keys()
       if conn_match in key_like:
           yield mapping[key_like]  # or return const or no-op (e.g., use the following break for the pass case)
           break
       else:
           continue

Rationale

While == is rather straight forward, a lot of complexity can creep into code from the use of in especially when performing partial matching. Yet, both conditions have overlap in how they can be simplified from "non-pythonic switch-like series of if-else" condition statements, into direct access mappings.

prerequisites that indicate when it is possible to simplify the code statement with in as done with ==:

• all comparisons are of pattern in some_constant
• all condition code blocks are either of the pattern return some_output_constant OR the pattern pass OR a single function call (e.g., func_match())

Example

This is an example where the mentioned rule(s) would currently be suboptimal:

	# anti-pattern
	def complex_triage_code(code: str) -> str:
		"Yields the severity of the given code."
		if not code:
			return "none"
		elif ("S" in code.upper()) or ("E2" in code.upper()) or ("E3" in code.upper()) or ("E9" in code.upper()):
			return "error"
		elif ("W" in code.upper()) or ("E" in code.upper()) or ("C" in code.upper()) or ("B" in code.upper()):
			return "warning"
		elif ("D" in code.upper()) or ("N" in code.upper()) or ("F" in code.upper()):
			return "note"
		else:
			return "none"


	# best practice
	def triage_code(code: str) -> str:
		"Yields the severity of the given code."
		if code:
			# use set for "or"-chains of conditions
			severity_map = {
				"error": {"S", "E2", "E3", "E9"},
				"warning": {"W", "E", "C", "B"},
				"note": {"D", "N", "F"}
			}
			# keep logic straight forward
			code_upper = code.upper()
			# use loop for nested sets of conditions instead of "or"-chains
			for severity, codes in severity_map.items():
				if any(code in code_upper for code in codes):
					return severity
		# default
		return "none"

# example test case
if __name__ in "test":
	test_case = "SIM" # arrange test

	assert(complex_triage_code(test_case) == triage_code(test_case))  # assert

	assert(triage_code(test_case) == complex_triage_code(test_case))  # assert

Rule SM116 does not identify cases with in instead of ==