Coverage for src/pullapprove/trust/imports.py: 89%

118 statements  

« prev     ^ index     » next       coverage.py v7.14.1, created at 2026-07-28 14:54 -0500

1"""Trust rule: hunks that only add, remove, or reorder import statements.""" 

2 

3from __future__ import annotations 

4 

5import re 

6 

7from ..diff import DiffCode, DiffFile, DiffHunk 

8from .helpers import change_suffix, extension 

9from .labels import Trust 

10from .linescan import collapse_ws 

11 

12# File extension -> (import prefixes, multi-line bracket or None for single-line). 

13IMPORT_CONFIG: dict[str, tuple[tuple[str, ...], str | None]] = { 

14 # `export { … }` is intentionally NOT here: adding/removing an export changes 

15 # the module's public API and must stay visible for review, unlike an import. 

16 **dict.fromkeys( 

17 "js jsx ts tsx mjs mts cjs cts".split(), 

18 (("import ", "import{"), "{"), 

19 ), 

20 "py": (("import ", "from "), "("), 

21 "go": (("import ",), "("), 

22 "rs": (("use ",), "{"), 

23 **dict.fromkeys("java kt kts scala groovy gradle".split(), (("import ",), None)), 

24 **dict.fromkeys("c cc cpp cxx h hpp m mm".split(), (("#include",), None)), 

25 "rb": (("require ", "require_relative "), None), 

26 "cs": (("using ",), None), 

27 **dict.fromkeys("swift dart".split(), (("import ",), None)), 

28} 

29 

30_CLOSING_BRACKET = {"(": ")", "{": "}"} 

31 

32# The extensions IMPORT_CONFIG groups under the JS/TS and C-family entries 

33# above, repeated here so the reorder and source-swap checks below can key off 

34# the language without re-deriving it from IMPORT_CONFIG's shared tuples. 

35_JS_TS_EXTS = frozenset("js jsx ts tsx mjs mts cjs cts".split()) 

36_C_EXTS = frozenset("c cc cpp cxx h hpp m mm".split()) 

37 

38 

39def _has_trailing_statement(text: str) -> bool: 

40 """True if a `;` separates more code, e.g. `import x; doThing()`. 

41 

42 A lone import ends at most with a trailing `;`; anything after the first one 

43 is a second statement we must not hide as import churn. 

44 """ 

45 semicolon = text.find(";") 

46 return semicolon != -1 and bool(text[semicolon + 1 :].strip()) 

47 

48 

49def _is_dynamic_import(text: str) -> bool: 

50 """A JS/TS dynamic `import(...)` call — `import` followed (after optional 

51 space) by `(`. It's executable code (lazy/conditional loading), not a static 

52 import declaration, so it must not be hidden as import churn.""" 

53 return text.startswith("import") and text[len("import") :].lstrip().startswith("(") 

54 

55 

56def _is_import_line(content: str, prefixes: tuple[str, ...]) -> bool: 

57 stripped = content.strip() 

58 return ( 

59 bool(stripped) 

60 and stripped.startswith(prefixes) 

61 and not _has_trailing_statement(stripped) 

62 and not _is_dynamic_import(stripped) 

63 ) 

64 

65 

66def _imports_only( 

67 lines: list[DiffCode], prefixes: tuple[str, ...], bracket: str | None 

68) -> bool: 

69 """True if this side is entirely import statements (handling multi-line).""" 

70 if bracket is None: 

71 return all( 

72 not line.content.strip() or _is_import_line(line.content, prefixes) 

73 for line in lines 

74 ) 

75 

76 # Heuristic, not a real parser: we track multi-line continuation by counting 

77 # brackets, which a bracket inside a string or comment would throw off. Import 

78 # statements rarely contain those, and a miscount only fails to label (safe). 

79 close = _CLOSING_BRACKET[bracket] 

80 depth = 0 

81 for line in lines: 

82 text = line.content.strip() 

83 if not text: 

84 continue 

85 # A JS/TS dynamic `import(...)` call is executable code, not a static 

86 # import. Skipped for `(`-bracket languages (Go), where `import (` opens a 

87 # grouped import rather than a call. 

88 if bracket != "(" and _is_dynamic_import(text): 

89 return False 

90 # No import line — including the one that closes a multi-line import 

91 # (which starts at depth > 0) — may carry a trailing statement, or the 

92 # executable code after the `;` would be hidden as import churn. 

93 if _has_trailing_statement(text): 

94 return False 

95 # Outside a bracketed block, every line must start a new import. 

96 if depth == 0 and not text.startswith(prefixes): 

97 return False 

98 depth += text.count(bracket) - text.count(close) 

99 if depth < 0: 

100 return False 

101 return depth == 0 

102 

103 

104def _sorted_imports(lines: list[DiffCode], bracket: str | None) -> list[str]: 

105 """The full import statements on this side, sorted — a bracket-spanned 

106 multi-line import is joined into one string so it compares by its complete 

107 text (members included), not just its opening line. (Callers pass only 

108 import-and-blank lines, guaranteed by `_imports_only`.)""" 

109 statements: list[str] = [] 

110 current: list[str] = [] 

111 depth = 0 

112 for line in lines: 

113 text = line.content.strip() 

114 if not text: 

115 continue 

116 current.append(text) 

117 if bracket is not None: 

118 depth += text.count(bracket) - text.count(_CLOSING_BRACKET[bracket]) 

119 if depth <= 0: # statement complete (single-line, or the closing bracket) 

120 statements.append(collapse_ws(" ".join(current))) 

121 current = [] 

122 depth = 0 

123 if current: # an unclosed trailing group (defensive; shouldn't occur) 

124 statements.append(collapse_ws(" ".join(current))) 

125 return sorted(statements) 

126 

127 

128def _is_import_reorder( 

129 added: list[DiffCode], removed: list[DiffCode], bracket: str | None 

130) -> bool: 

131 """Same set of whole import statements, just in a different order (an empty 

132 result on both sides is not a reorder — there's nothing to reorder).""" 

133 added_imports = _sorted_imports(added, bracket) 

134 return bool(added_imports) and added_imports == _sorted_imports(removed, bracket) 

135 

136 

137def _is_side_effect_import(statement: str) -> bool: 

138 """A JS/TS side-effect import — `import 'spec';` — binds no names. Unlike 

139 `import x from 'spec'`, its only purpose is running the module's top-level 

140 code, so two of them (`import './polyfills'; import './init-sentry';`) can 

141 depend on running in a particular order even though the *set* of specifiers 

142 is unchanged.""" 

143 after_import = statement[len("import") :].lstrip() 

144 return after_import.startswith(("'", '"')) 

145 

146 

147def _reorder_is_trivial(statements: list[str], ext: str) -> bool: 

148 """False when reordering these (already-confirmed-identical-as-a-set) 

149 import statements can change behavior, so the reorder must still be 

150 declined rather than trusted. 

151 

152 - C/C++/Obj-C `#include`: can define macros/typedefs or trigger conditional 

153 compilation that a later include depends on — order is always 

154 potentially meaningful, so reorders are never trusted for this family. 

155 - JS/TS: trivial unless the reordered lines include a bare side-effect 

156 import (see `_is_side_effect_import`). 

157 """ 

158 if ext in _C_EXTS: 

159 return False 

160 if ext in _JS_TS_EXTS: 

161 return not any(_is_side_effect_import(stmt) for stmt in statements) 

162 return True 

163 

164 

165# Languages where `_import_source` can reliably split "module" from "names" — 

166# i.e. a paired add/remove commonly keeps the module and only varies the 

167# imported names. Extending this to every configured language is out of scope 

168# for now; for the rest, `_same_import_sources` can't tell a name-only edit 

169# from a module swap, so it stays permissive (matching the prior behavior). 

170_SOURCE_AWARE_EXTS = frozenset({"py"}) | _JS_TS_EXTS | _C_EXTS 

171 

172 

173def _import_source(statement: str, ext: str) -> str | None: 

174 """The module/path this single import statement pulls from, or None if we 

175 can't confidently identify it (fail safe: an unparsed statement must not 

176 be treated as matching another one).""" 

177 if ext == "py": 

178 if statement.startswith("from "): 

179 module, _, _ = statement[len("from ") :].partition(" import") 

180 return module.strip() or None 

181 if statement.startswith("import "): 

182 token = re.split(r"[,\s]", statement[len("import ") :], maxsplit=1)[0] 

183 return token or None 

184 return None 

185 if ext in _JS_TS_EXTS: 

186 match = re.search(r"""from\s*(['"])(.*?)\1""", statement) 

187 if match: 

188 return match.group(2) 

189 # No `from` clause: a bare side-effect import, where the quoted spec 

190 # itself IS the source (`import 'spec';`). 

191 after_import = statement[len("import") :].lstrip() 

192 match = re.match(r"""(['"])(.*?)\1""", after_import) 

193 return match.group(2) if match else None 

194 if ext in _C_EXTS: 

195 match = re.search(r'["<]([^">]+)[">]', statement) 

196 return match.group(1) if match else None 

197 return None 

198 

199 

200def _same_import_sources( 

201 added: list[DiffCode], removed: list[DiffCode], bracket: str | None, ext: str 

202) -> bool: 

203 """True only if every added and removed import statement resolves to the 

204 same set of source modules — i.e. the hunk adds/removes names within an 

205 unchanged set of modules, not a swap to a different module 

206 (`-from foo import bar` / `+from evil import bar`).""" 

207 if ext not in _SOURCE_AWARE_EXTS: 

208 # We can't split "module" from "names" for this language, so we can't 

209 # confirm a paired modification keeps the same source rather than 

210 # swapping to a different module — e.g. Ruby `require 'safe'` -> 

211 # `require 'evil'`, which executes a different file. Decline (show it) 

212 # rather than trust the swap blindly; pure add/remove/reorder still 

213 # label via their own paths. 

214 return False 

215 added_sources = {_import_source(s, ext) for s in _sorted_imports(added, bracket)} 

216 removed_sources = { 

217 _import_source(s, ext) for s in _sorted_imports(removed, bracket) 

218 } 

219 # A statement we couldn't parse (None) is a hard stop — fail safe, don't trust. 

220 if None in added_sources or None in removed_sources: 

221 return False 

222 return added_sources == removed_sources 

223 

224 

225def _imports(file: DiffFile, hunk: DiffHunk) -> Trust | None: 

226 ext = extension(file) 

227 config = IMPORT_CONFIG.get(ext) 

228 if not config: 

229 return None 

230 prefixes, bracket = config 

231 added, removed = hunk.added_lines, hunk.removed_lines 

232 if not added and not removed: 

233 return None 

234 # `from __future__ import …` is a compiler directive, not a plain import: 

235 # adding one changes the whole module's semantics, and moving one below 

236 # another import is a SyntaxError — never trivial, so decline the hunk. 

237 if any("__future__" in line.content for line in (*added, *removed)): 

238 return None 

239 if not _imports_only(added, prefixes, bracket) or not _imports_only( 

240 removed, prefixes, bracket 

241 ): 

242 return None 

243 if added and removed and _is_import_reorder(added, removed, bracket): 

244 # Same statements, different order. Whether that's trustworthy depends 

245 # on the language — if not, decline outright rather than falling 

246 # through to the "modified" check below, which only compares source 

247 # modules and would trivially pass a same-statement reorder. 

248 statements = _sorted_imports(added, bracket) 

249 if not _reorder_is_trivial(statements, ext): 

250 return None 

251 return Trust.IMPORTS_REORDERED 

252 suffix = change_suffix(hunk) 

253 # A "modified" hunk (both sides non-empty, not a pure reorder) is only 

254 # trustworthy when it's adding/removing names within the same set of 

255 # source modules — not swapping to a different module. 

256 if suffix == "modified" and not _same_import_sources(added, removed, bracket, ext): 

257 return None 

258 # change_suffix yields added/removed/modified — each a real Trust member. 

259 return Trust(f"imports:{suffix}")