|
| 1 | +#!/usr/bin/env python3 |
| 2 | +# Licensed to the Apache Software Foundation (ASF) under one |
| 3 | +# or more contributor license agreements. See the NOTICE file |
| 4 | +# distributed with this work for additional information |
| 5 | +# regarding copyright ownership. The ASF licenses this file |
| 6 | +# to you under the Apache License, Version 2.0 (the |
| 7 | +# "License"); you may not use this file except in compliance |
| 8 | +# with the License. You may obtain a copy of the License at |
| 9 | +# |
| 10 | +# http://www.apache.org/licenses/LICENSE-2.0 |
| 11 | +# |
| 12 | +# Unless required by applicable law or agreed to in writing, |
| 13 | +# software distributed under the License is distributed on an |
| 14 | +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY |
| 15 | +# KIND, either express or implied. See the License for the |
| 16 | +# specific language governing permissions and limitations |
| 17 | +# under the License. |
| 18 | +# |
| 19 | +# Checks @APICommand/@Parameter/@Param `since` usage on branches that have |
| 20 | +# moved past the old 4.x version scheme (i.e. project version >= 24): |
| 21 | +# |
| 22 | +# 1. A newly added since="4.x" is flagged - contributors often add this out |
| 23 | +# of muscle memory even though the project is now versioned e.g. 24.0.0. |
| 24 | +# 2. A brand-new @APICommand/@Parameter/@Param (its annotation AND the |
| 25 | +# class/field it annotates are both newly added together) that has no |
| 26 | +# since attribute at all is flagged - new API surface should record when |
| 27 | +# it was introduced. Editing an existing annotation (its declaration |
| 28 | +# line is not part of the diff) never requires since, even if the |
| 29 | +# existing element never had one. |
| 30 | +# |
| 31 | +# Only lines actually added by the PR are inspected. A value/field is |
| 32 | +# ignored if the same PR also removes the identical since="..." value, or |
| 33 | +# the identical field/class name, elsewhere in the same file's diff - this |
| 34 | +# covers a field being moved or reformatted rather than a genuinely new |
| 35 | +# API/param/response field. |
| 36 | + |
| 37 | +import argparse |
| 38 | +import re |
| 39 | +import subprocess |
| 40 | +import sys |
| 41 | + |
| 42 | +ANNOTATION_START_RE = re.compile(r"@(Param|Parameter|APICommand)\s*\(") |
| 43 | +SINCE_RE = re.compile(r'since\s*=\s*"(4\.\d[\w.]*)"') |
| 44 | +HAS_SINCE_RE = re.compile(r"\bsince\s*=") |
| 45 | +VERSION_RE = re.compile(r"<artifactId>cloudstack</artifactId>\s*<version>([^<]+)</version>") |
| 46 | +FIELD_DECL_RE = re.compile(r"^\s*(?:private|protected|public)\b[^=;(){}]*?(\w+)\s*;\s*$") |
| 47 | +CLASS_DECL_RE = re.compile(r"^\s*(?:public\s+)?(?:final\s+)?class\s+(\w+)") |
| 48 | + |
| 49 | + |
| 50 | +def read_project_version(pom_path: str) -> str: |
| 51 | + with open(pom_path, encoding="utf-8") as f: |
| 52 | + content = f.read() |
| 53 | + match = VERSION_RE.search(content) |
| 54 | + if not match: |
| 55 | + raise SystemExit(f"Could not find the cloudstack project version in {pom_path}") |
| 56 | + return match.group(1) |
| 57 | + |
| 58 | + |
| 59 | +def major_version(version: str) -> int: |
| 60 | + match = re.match(r"(\d+)", version) |
| 61 | + if not match: |
| 62 | + raise SystemExit(f"Could not parse a major version from '{version}'") |
| 63 | + return int(match.group(1)) |
| 64 | + |
| 65 | + |
| 66 | +def git_diff(base: str, head: str) -> str: |
| 67 | + return subprocess.run( |
| 68 | + ["git", "diff", "--no-color", "--unified=0", base, head, "--", "*.java"], |
| 69 | + check=True, |
| 70 | + capture_output=True, |
| 71 | + text=True, |
| 72 | + ).stdout |
| 73 | + |
| 74 | + |
| 75 | +def diff_path(line: str) -> str: |
| 76 | + path = line[4:] |
| 77 | + return path[2:] if path.startswith(("a/", "b/")) else path |
| 78 | + |
| 79 | + |
| 80 | +def parse_hunks(diff_text: str) -> dict: |
| 81 | + """file -> list of hunks, each hunk = {"added": [lines], "removed": [lines]}""" |
| 82 | + files: dict = {} |
| 83 | + current_file = None |
| 84 | + current_hunk = None |
| 85 | + for line in diff_text.splitlines(): |
| 86 | + if line.startswith("+++ "): |
| 87 | + current_file = diff_path(line) |
| 88 | + files.setdefault(current_file, []) |
| 89 | + current_hunk = None |
| 90 | + elif line.startswith("--- "): |
| 91 | + continue |
| 92 | + elif line.startswith("@@"): |
| 93 | + current_hunk = {"added": [], "removed": []} |
| 94 | + files[current_file].append(current_hunk) |
| 95 | + elif current_hunk is not None: |
| 96 | + if line.startswith("+") and not line.startswith("+++"): |
| 97 | + current_hunk["added"].append(line[1:]) |
| 98 | + elif line.startswith("-") and not line.startswith("---"): |
| 99 | + current_hunk["removed"].append(line[1:]) |
| 100 | + return files |
| 101 | + |
| 102 | + |
| 103 | +def file_removed_text(hunks: list) -> str: |
| 104 | + return "\n".join(l for h in hunks for l in h["removed"]) |
| 105 | + |
| 106 | + |
| 107 | +def find_old_scheme_violations(files: dict) -> list: |
| 108 | + violations = [] |
| 109 | + for path, hunks in files.items(): |
| 110 | + removed_values = set(SINCE_RE.findall(file_removed_text(hunks))) |
| 111 | + for hunk in hunks: |
| 112 | + for line in hunk["added"]: |
| 113 | + for value in SINCE_RE.findall(line): |
| 114 | + if value in removed_values: |
| 115 | + continue |
| 116 | + violations.append(("old_scheme", path, line.strip(), value)) |
| 117 | + return violations |
| 118 | + |
| 119 | + |
| 120 | +def find_matching_paren(text: str, open_pos: int) -> int: |
| 121 | + depth = 1 |
| 122 | + i = open_pos + 1 |
| 123 | + while i < len(text) and depth: |
| 124 | + if text[i] == "(": |
| 125 | + depth += 1 |
| 126 | + elif text[i] == ")": |
| 127 | + depth -= 1 |
| 128 | + i += 1 |
| 129 | + return i - 1 if depth == 0 else -1 |
| 130 | + |
| 131 | + |
| 132 | +def find_missing_since_violations(files: dict) -> list: |
| 133 | + violations = [] |
| 134 | + for path, hunks in files.items(): |
| 135 | + removed_text = file_removed_text(hunks) |
| 136 | + for hunk in hunks: |
| 137 | + joined = "\n".join(hunk["added"]) |
| 138 | + for match in ANNOTATION_START_RE.finditer(joined): |
| 139 | + kind = match.group(1) |
| 140 | + open_pos = match.end() - 1 |
| 141 | + close_pos = find_matching_paren(joined, open_pos) |
| 142 | + if close_pos == -1: |
| 143 | + continue # annotation not fully contained in this hunk; can't tell, skip |
| 144 | + annotation_text = joined[match.start():close_pos + 1] |
| 145 | + if HAS_SINCE_RE.search(annotation_text): |
| 146 | + continue # has since (old-scheme check handles wrong values separately) |
| 147 | + |
| 148 | + remainder = joined[close_pos + 1:] |
| 149 | + decl_re = CLASS_DECL_RE if kind == "APICommand" else FIELD_DECL_RE |
| 150 | + decl_match = None |
| 151 | + for candidate in remainder.splitlines(): |
| 152 | + candidate = candidate.strip() |
| 153 | + if not candidate: |
| 154 | + continue |
| 155 | + decl_match = decl_re.match(candidate) |
| 156 | + break # only look at the next non-blank added line |
| 157 | + |
| 158 | + if not decl_match: |
| 159 | + continue # declaration wasn't (re)added alongside the annotation -> a modification, not new |
| 160 | + |
| 161 | + name = decl_match.group(1) |
| 162 | + if re.search(rf"\b{re.escape(name)}\b\s*[;{{]", removed_text): |
| 163 | + continue # same name also removed elsewhere in this file's diff -> likely a move/reformat |
| 164 | + |
| 165 | + violations.append(("missing_since", path, annotation_text.strip(), kind)) |
| 166 | + return violations |
| 167 | + |
| 168 | + |
| 169 | +def main() -> int: |
| 170 | + parser = argparse.ArgumentParser(description=__doc__) |
| 171 | + parser.add_argument("--pom", default="pom.xml") |
| 172 | + parser.add_argument("--base", required=True, help="Base commit SHA of the PR") |
| 173 | + parser.add_argument("--head", required=True, help="Head commit SHA of the PR") |
| 174 | + args = parser.parse_args() |
| 175 | + |
| 176 | + version = read_project_version(args.pom) |
| 177 | + major = major_version(version) |
| 178 | + print(f"Project version from {args.pom}: {version} (major: {major})") |
| 179 | + |
| 180 | + if major < 24: |
| 181 | + print( |
| 182 | + "Project major version is below 24; API annotations still use the " |
| 183 | + "4.x 'since' scheme on this branch. Skipping check." |
| 184 | + ) |
| 185 | + return 0 |
| 186 | + |
| 187 | + diff_text = git_diff(args.base, args.head) |
| 188 | + files = parse_hunks(diff_text) |
| 189 | + |
| 190 | + old_scheme = find_old_scheme_violations(files) |
| 191 | + missing_since = find_missing_since_violations(files) |
| 192 | + |
| 193 | + if not old_scheme and not missing_since: |
| 194 | + print("No newly added/changed API annotations have a 'since' problem.") |
| 195 | + return 0 |
| 196 | + |
| 197 | + for _, path, line, value in old_scheme: |
| 198 | + print( |
| 199 | + f'::error file={path}::since="{value}" uses the pre-24 CloudStack versioning scheme. ' |
| 200 | + f'This project is now versioned {version}; new @APICommand/@Parameter/@Param annotations ' |
| 201 | + f'should use since="{major}.x" (e.g. "{major}.0") instead. Offending line: {line}' |
| 202 | + ) |
| 203 | + |
| 204 | + for _, path, annotation_text, kind in missing_since: |
| 205 | + snippet = " ".join(annotation_text.split()) |
| 206 | + print( |
| 207 | + f"::error file={path}::A newly added @{kind} is missing a 'since' attribute. " |
| 208 | + f'New API commands/params/response fields should record when they were introduced, ' |
| 209 | + f'e.g. since="{major}.0". Annotation: {snippet}' |
| 210 | + ) |
| 211 | + |
| 212 | + total = len(old_scheme) + len(missing_since) |
| 213 | + print(f"\n{total} issue(s) found ({len(old_scheme)} outdated 4.x value(s), {len(missing_since)} missing since).") |
| 214 | + print( |
| 215 | + "Note: this only flags brand-new annotations (added together with the class/field they " |
| 216 | + "annotate) and newly added since values; editing an existing annotation never requires " |
| 217 | + "adding since, and a moved/reformatted field is detected by its name also appearing on a " |
| 218 | + "removed line in the same file and is not flagged." |
| 219 | + ) |
| 220 | + return 1 |
| 221 | + |
| 222 | + |
| 223 | +if __name__ == "__main__": |
| 224 | + sys.exit(main()) |
0 commit comments