清單：從規(guī)范到架構(gòu)的核心檢查點)
把幾十個 Python 項目的 merge request 從頭到尾過一遍之后我對代碼審查的態(tài)度經(jīng)歷了一個反轉(zhuǎn)它不只是一道“檢查風(fēng)格是否符合規(guī)范”的關(guān)卡更是一個人把自己的判斷力注入項目的好機會。這篇文章不打算羅列教科書式的規(guī)范條目而是從實際 review 里沉淀下來的一套 Python 代碼審查實戰(zhàn)檢查清單——從最基本的編碼規(guī)范一路看到系統(tǒng)架構(gòu)每個檢查點都講清楚為什么要看、怎么看、以及我踩過什么樣的坑。適合剛接手 MR 評審的同學(xué)也適合想在提交前自己做一遍自檢的開發(fā)者。1. 代碼審查到底在看什么先分清四層目標(biāo)1.1 規(guī)范、正確性、設(shè)計與演進我最早做 review 的時候滿腦子都是“有沒有違反 PEP 8”“變量名是不是規(guī)范”結(jié)果把時間都花在了挑格式上真正有問題的邏輯反而被放過去了。后來我把 review 的目標(biāo)拆成四層每一層的審查方式和語氣都不一樣。規(guī)范層代碼能不能被人類順暢地讀懂命名是否誠實函數(shù)是否短到可以一眼看完。正確性層邊界條件、異常路徑、資源釋放、并發(fā)狀態(tài)在真實運行中會不會出問題。設(shè)計層模塊劃分是否合理依賴方向是否清晰新代碼是強化了既有結(jié)構(gòu)還是繞過了它。演進層這次改動三個月后會不會變成債接口是否向后兼容別人是否敢在這個基礎(chǔ)上繼續(xù)改。這四層的關(guān)系可以用下面這張表來理解層次關(guān)注點典型問題規(guī)范層可讀性、命名、結(jié)構(gòu)函數(shù)太長、命名誤導(dǎo)、邏輯藏在注釋里正確性邊界、異常、資源、競態(tài)空值崩潰、資源泄漏、重試不冪等設(shè)計層模塊劃分、依賴方向循環(huán) import、控制器里塞業(yè)務(wù)、工具模塊反向依賴演進層兼容性、擴展成本改簽名、動返回結(jié)構(gòu)、臨時補丁沒人認領(lǐng)每次 review 之前先問自己這次提交屬于哪一層大多數(shù)提交同時涉及多個層次但你的注意力分配不一樣。一個腳本實驗只要規(guī)范層過得去就行核心支付鏈路上的改動則必須把正確性和演進層都審到。換句話說代碼審查不是走流程而是先用“這層代碼會活多久”來判斷投入的精力。1.2 Diff 閱讀和全量閱讀的取舍很多人打開 MR 只看 diff逐行看新增代碼就結(jié)束了。我承認 diff 是高效手段但只靠 diff 會漏掉一類重要信息改動和周邊代碼的交互方式。我的做法是改動量在 200 行以內(nèi)把涉及的文件全量讀一遍超過 300 行先要求提交者把 MR 拆小拆不開的至少給出“核心改動”和“配套改動”的說明。全量閱讀能發(fā)現(xiàn)那種“新增函數(shù)看似通用其實和某個舊邏輯重復(fù)”的問題這類問題在 diff 里幾乎看不出來。另一個經(jīng)驗是如果提交者連 commit message 都寫不清“為什么改”那大概率連他自己都沒想清楚。review 從 commit message 開始能省掉后面大半的溝通成本。1.3 讀代碼的順序先接口再實現(xiàn)最后測試我推薦一套固定的閱讀順序先看公開函數(shù)的簽名和類型注解再讀實現(xiàn)里的異常分支最后看測試有沒有覆蓋關(guān)鍵路徑。先看簽名能快速判斷設(shè)計意圖比如get_user(user_id: int) - User | None比get_user(user_id)能傳遞的信息多得多看實現(xiàn)時重點盯“如果輸入非法會怎樣”而不是“正常輸入能不能跑通”最后看測試不是為了看覆蓋率數(shù)字而是確認提交者是否真的考慮過異常場景。這個順序能阻止你陷入局部細節(jié)。很多時候review 到一半突然發(fā)現(xiàn)接口設(shè)計本身就是錯的那后面幾十行格式問題已經(jīng)不值得再看了。2. 規(guī)范層PEP 8 之外真正影響可讀性的細節(jié)2.1 命名是低成本的架構(gòu)決策PEP 8 規(guī)定了命名風(fēng)格但沒規(guī)定命名質(zhì)量。我在 review 里看到最多的不是“命名不符合規(guī)范”而是“命名根本沒表達含義”。# 反例任何人都猜不到 process_data 具體做什么 def process_data(data): ... # 推薦函數(shù)名直接回答“做什么” def normalize_phone_number(raw: str) - str: ...data、item、temp、result這類變量名不是不能用而是出現(xiàn)頻率太高時讀代碼的人就得反復(fù)跳到定義處才能確認含義。每跳一次認知負荷就高一截。review 時看到新的變量名我會問自己一個新同事不看上下文能不能在 3 秒內(nèi)說出這個變量存的是什么如果不行這個命名就需要改。2.2 類型注解從文檔注釋到可執(zhí)行約束Python 是動態(tài)語言但團隊項目不應(yīng)該把“動態(tài)”當(dāng)作不做類型標(biāo)注的理由。類型注解最大的價值不是好看而是讓錯誤的可能性變窄同時給調(diào)用方一份不需要翻源碼的契約。# 不推薦 def get_user(user_id): return db.query(fSELECT * FROM users WHERE id {user_id}) # 推薦 def get_user(user_id: int) - User | None: return db.query_one(User, iduser_id)review 時看到# type: ignore我不會直接放過去而是按“這里藏著一個解釋不清的問題”來處理。它可能是第三方庫的 isue也可能是類型設(shè)計本身有問題。對于 Python 3.10 以下的團隊int | None要寫成Optional[int]這點在 review 時要根據(jù)項目版本保持一致。注解也不是越多越好。內(nèi)部函數(shù)和私有方法可以少注解公開接口必須完整尤其是返回類型。返回類型缺失調(diào)用方拿到什么都只能靠猜這是規(guī)范層里最隱蔽的成本。2.3 函數(shù)長度與認知負荷“一個函數(shù)最好不超過 50 行”這類建議太絕對我更喜歡用“一眼定律”打開函數(shù)視線掃一遍能不能看出它總共在干幾件事。如果一個函數(shù)做到了三件事解析輸入、調(diào)外部服務(wù)、寫緩存那么即使只有 30 行也應(yīng)該拆。相反一個線性執(zhí)行的純計算函數(shù)長一點也沒太大問題因為它的邏輯是連續(xù)的。真正危險的是多層縮進、循環(huán)套條件、條件里再調(diào)函數(shù)這種結(jié)構(gòu)讓每一行的狀態(tài)都依賴前面的狀態(tài)讀起來像是在做思維迷宮。review 時看到超過 50 行且多層嵌套的函數(shù)第一反應(yīng)不是“給我拆成更短”而是“它同時在干幾件事”。讓提交者把職責(zé)拆開往往比單純要求縮短更有用。2.4 自動化工具能做的別讓人眼做格式、未使用的變量、import 順序、明顯的語法問題這些全部應(yīng)該交給工具。我的人工注意力只留給工具判斷不了的問題語義、邊界、依賴、演進。一個可復(fù)制的組合是用自動格式化工具統(tǒng)一代碼風(fēng)格用靜態(tài)檢查工具掃常見問題用類型檢查工具驗證注解最后用統(tǒng)一配置的 pre-commit 在提交前攔截。團隊里最想避免的是每個開發(fā)者本地配置不同導(dǎo)致 MR 里全是格式噪音真正的邏輯問題反而被淹沒。檢查項交給工具必須人眼縮進、行寬、import 排序是否未使用變量、未定義名稱是否類型不匹配是否命名是否誤導(dǎo)否是注釋是否與代碼同步過時否是函數(shù)職責(zé)是否單一否是3. 正確性與異常路徑最容易放走的隱蔽 Bug3.1 空值、邊界和“看起來不可能”的輸入Review 里最常漏掉的 bug往往出現(xiàn)在“正常路徑?jīng)]有問題極端輸入直接崩潰”的地方。列表分片、字典取值、鏈?zhǔn)秸{(diào)用都是重災(zāi)區(qū)。# 看起來沒問題但 items 為空時會發(fā)生什么 batch_size len(items) // 3 for chunk in [items[i:i batch_size] for i in range(0, len(items), batch_size)]: process(chunk)當(dāng)items為空時batch_size是 0range(0, 0, 0)會直接拋ValueError。這個例子說明review 時看到列表運算至少要在心里做一遍“空列表、單元素列表、n 不能被整除”的推演。我自己在 review 訂單回調(diào)邏輯時遇到過類似問題生產(chǎn)環(huán)境里數(shù)據(jù)量偶爾為 0測試數(shù)據(jù)從沒覆蓋上線后第一分鐘就報警。字典的鏈?zhǔn)饺≈狄彩峭瑯拥倪壿媟esult data.get(key, {}).get(value, 0)如果data[key]存在但值是None這一行照樣拋AttributeError。review 時看到鏈?zhǔn)饺≈叼B(yǎng)成習(xí)慣問一句中間節(jié)點可能為 None 嗎3.2 異常粒度什么時候 catch什么時候讓它拋出去except Exception是 Python 項目里最讓人頭疼的寫法之一。它看起來安全實際上把所有異常都吞掉了包括代碼里真正的 bug。# 反例失敗時完全不知道發(fā)生了什么 try: response requests.post(url, jsonpayload, timeout5) except Exception: pass # 推薦區(qū)分異常類型并保留原始上下文 try: response requests.post(url, jsonpayload, timeout5) except requests.Timeout as exc: raise ExternalServiceTimeout(payment service timeout) from exc except requests.RequestException as exc: raise ExternalServiceError(payment service unavailable) from excraise ... from exc不是形式主義它能把底層異常鏈完整保留排查問題時少走很多彎路。review 時看到裸except: pass基本可以直接打回。如果確實只需要兜底日志至少要用logger.exception(...)記錄完整堆棧而不是讓異常靜默消失。3.3 資源與上下文with 之外的隱藏狀態(tài)Python 程序員都知道with open(...)可以安全處理文件但資源管理的坑遠不止文件。數(shù)據(jù)庫連接、線程鎖、臨時目錄、信號量、緩存客戶端都需要明確的生命周期。# 不推薦acquire 和 release 隔著 20 行中間任何 return 都會漏釋放 lock.acquire() try: ... finally: lock.release() # 推薦用上下文管理器收斂生命周期 with lock: ...review 時看到acquire()和release()相距很遠必須警惕。這類代碼一旦中間出現(xiàn)提前 return 或者新異常鎖就永遠放不掉了。更好的方式是把資源生命周期收進with語句或顯式的上下文管理器里讓“進入和退出”成對出現(xiàn)。另外一個容易被忽略的資源是緩存和連接池。比如每請求創(chuàng)建新的數(shù)據(jù)庫連接而不是復(fù)用連接池這在低并發(fā)時沒事流量上來就直接打滿連接數(shù)。3.4 按“發(fā)生中斷后怎么辦”來讀正確性 review 的最高標(biāo)準(zhǔn)不是正常流程是否能跑通而是每一步“發(fā)生中斷后系統(tǒng)是否還能自洽”。舉個例子處理支付回調(diào)時是先改訂單狀態(tài)還是先記賬如果改完狀態(tài)、記賬之前進程崩了重試會發(fā)生什么訂單會不會被重復(fù)記賬如果調(diào)用外部接口超時重試邏輯會不會把同樣的請求發(fā)兩次這類問題在 happy path 里看不見但生產(chǎn)環(huán)境里幾乎天天發(fā)生。review 時看到寫數(shù)據(jù)庫、發(fā)消息、調(diào)外部服務(wù)的代碼我會要求提交者說明“重試一次會發(fā)生什么”。如果對方答不上來說明這個改動還沒想清楚。冪等鍵、去重表、狀態(tài)機限制都是這類問題的常見解法但更重要的是先把問題問出來。4. 性能與并發(fā)Review 時如何定位潛在瓶頸4.1 復(fù)雜度預(yù)判先看循環(huán)嵌套和數(shù)據(jù)規(guī)模性能 review 不是靠猜而是靠量級判斷。看到嵌套循環(huán)時我會先確認數(shù)據(jù)規(guī)模外層 10 條、內(nèi)層 1000 條和內(nèi)外都是 10 萬條處理方式完全不同。# O(n*m)每個元素都在列表里做一次線性查找 for user_id in user_ids: if user_id in all_user_ids: ... # O(nm)先用集合建立索引 known_ids set(all_user_ids) for user_id in user_ids: if user_id in known_ids: ...把列表in查找換成集合或字典是 Python 項目里性價比最高的性能改進。但 review 時也要克制如果數(shù)據(jù)規(guī)模只有幾百這種優(yōu)化對用戶毫無感知反而讓代碼多幾行。性能問題的優(yōu)先級應(yīng)該由“實際量級”決定而不是由“聽起來很慢”決定。4.2 數(shù)據(jù)庫查詢和 IO 重災(zāi)區(qū)會延續(xù)Python Web 服務(wù)里最常見的性能問題是 N1 查詢外層循環(huán)查詢主記錄循環(huán)里再一次次查關(guān)聯(lián)數(shù)據(jù)。review 時看到循環(huán)里執(zhí)行數(shù)據(jù)庫查詢直接標(biāo)記。# N1 查詢 for order in orders: items.append(get_items_for_order(order.id)) # 批量查詢 items get_items_for_orders([o.id for o in orders])同樣的道理適用于循環(huán)里發(fā) HTTP 請求、循環(huán)里寫文件、循環(huán)里調(diào)第三方 SDK。每一次 IO 都有網(wǎng)絡(luò)延遲和連接開銷批量合并往往能帶來數(shù)量級的提升。review 時還要看事務(wù)邊界長事務(wù)里做外部調(diào)用會讓數(shù)據(jù)庫連接長時間占用連接池一旦耗盡整個服務(wù)都會受影響。4.3 多線程與異步可重入、競態(tài)與阻塞多線程代碼在 review 里最難發(fā)現(xiàn)的是競態(tài)問題。比如一個看似簡單的計數(shù)器counter 0 def worker(): global counter for _ in range(100): counter 1counter 1不是原子操作線程交替執(zhí)行時會出現(xiàn)“讀-算-寫”中間被切換的情況。review 時看到共享狀態(tài)被多線程讀寫要么加鎖要么用queue或concurrent.futures的線程安全結(jié)構(gòu)。異步代碼也有自己的坑。在async函數(shù)里調(diào)用同步阻塞 IO會直接卡住整個事件循環(huán)導(dǎo)致其他請求一起變慢。review 時看到await函數(shù)里混著time.sleep或同步requests應(yīng)該提示改成異步版本或者放到線程池里執(zhí)行。4.4 沒有 profile 不下定論性能 review 最容易犯的錯誤是憑直覺要求“優(yōu)化”。有一次我 review 一個數(shù)據(jù)校驗功能建議把某處的list換成set理由是從復(fù)雜度上看更快。后來提交者實際測試數(shù)據(jù)量只有幾百條改動前后耗時差異在誤差范圍內(nèi)反而因為引入集合的哈希約束代碼可讀性變差了。從那以后我給自己定了一條紀(jì)律review 時可以指出明顯的復(fù)雜度問題但涉及微優(yōu)化必須給出可驗證的收益。通常先問“這里的數(shù)據(jù)量級是多少”“有沒有壓測數(shù)據(jù)”再決定要不要動。5. 安全與依賴代碼審查中的風(fēng)險掃描5.1 注入與路徑操作把不可信輸入當(dāng)敵人Python 里最容易出現(xiàn)的注入是 SQL 注入和命令注入。review 時看到字符串拼接 SQL無論提交者怎么解釋都先假定是有問題的。# 反例用戶輸入直接拼進 SQL query fSELECT * FROM users WHERE name {name} # 正例參數(shù)化查詢 query SELECT * FROM users WHERE name ?文件路徑也一樣。用戶傳入的文件名如果包含../直接拼進os.path.join(base_dir, filename)就能越界讀到其他目錄。review 時看到用用戶輸入構(gòu)造路徑至少要用Path.resolve()校驗最終路徑是否仍然位于允許的目錄內(nèi)。命令注入則藏在subprocess.run(fls {path}, shellTrue)這類寫法里。能不用shellTrue就不用改用參數(shù)列表傳參是更安全的方式。5.2 敏感信息密鑰、Token 與日志泄露很多密鑰事故不是發(fā)生在配置中心而是發(fā)生在日志里。review 時看到logger.info(request failed: %s, request_body)要考慮request_body里是否包含密碼、Token、手機號一類信息。一個很常見的場景是開發(fā)環(huán)境調(diào)試時為了方便把整個請求體打到日志里上線的時候忘了刪。我的檢查習(xí)慣是全文搜索幾個關(guān)鍵詞password、token、secret、api_key看它們有沒有出現(xiàn)在日志輸出、錯誤消息、URL 參數(shù)和數(shù)據(jù)庫字段里。如果確實需要記錄請求體至少要做脫敏處理比如把密碼字段替換成***。5.3 依賴鎖定與第三方風(fēng)險requirements.txt里寫package1.0等于把版本命運交給未來。review 時看到依賴沒有鎖定版本或者鎖定版本但不鎖定傳遞依賴我會要求改成固定版本加鎖文件的方式保證任何人在任何時候拉代碼裝出來的環(huán)境是一致的。另一個常被忽略的問題是直接從一個 Git 倉庫pip install git...安裝依賴。這不一定是錯的但必須固定到具體 commit 或者 tag否則第三方倉庫的每次推送都會改變你的構(gòu)建結(jié)果。定期掃描已安裝依賴的已知漏洞也應(yīng)該進入團隊的例行任務(wù)。5.4 服務(wù)端校驗與權(quán)限前端校驗只是用戶體驗服務(wù)端校驗才是安全邊界。review 時看到新接口沒有校驗必填字段、長度、格式直接打回。尤其要注意接口是否真的做了權(quán)限判斷而不是只依賴網(wǎng)關(guān)過濾。同事曾經(jīng)在網(wǎng)關(guān)層做了 auth 校驗業(yè)務(wù)層就默認所有請求都合法直到有人發(fā)現(xiàn)一個內(nèi)部接口可以被普通用戶直接調(diào)用改了好幾天才補完。6. 架構(gòu)層面這次改動是否在給系統(tǒng)挖坑6.1 依賴方向與分層讓數(shù)據(jù)流單向流動代碼審查要看的最后一個大層次是這次改動對整體結(jié)構(gòu)的影響。我最常遇到的架構(gòu)問題是循環(huán) import 和分層混亂。一個典型的信號是視圖函數(shù)里寫業(yè)務(wù)邏輯、業(yè)務(wù)函數(shù)里拼 SQL、而工具模塊反過來 import 業(yè)務(wù)模塊。數(shù)據(jù)流一旦開始繞圈任何改動都有可能踩到隱藏的耦合。review 時如果發(fā)現(xiàn)自己畫不出“這次改動從哪里來、要往哪里去”的依賴關(guān)系就應(yīng)該打回去要求重構(gòu)。推薦的做法是讓依賴方向保持單向控制層調(diào)服務(wù)層服務(wù)層調(diào)數(shù)據(jù)訪問層工具模塊只提供無業(yè)務(wù)含義的通用能力不反向依賴業(yè)務(wù)。分層不是教條而是讓未來改任何一層的成本都可控。6.2 接口設(shè)計與向后兼容改函數(shù)簽名、改字典 key、改返回類型意味著所有調(diào)用方都要跟著變。review 時看到這類改動第一件事是在倉庫里搜索所有調(diào)用點確認影響范圍。# 原來返回 dict調(diào)用方用 config[timeout] 獲取值 def get_config() - dict: ... # 改成返回 dataclass調(diào)用方原來的寫法全部失效 def get_config() - AppConfig: ...如果這是內(nèi)部接口可以保留舊函數(shù)并加 deprecated 警告先讓調(diào)用方遷移再在下一個版本移除。如果是對外 HTTP API優(yōu)先考慮新增版本而不是原地修改。一次“順手”的接口變更可能讓下游系統(tǒng)凌晨炸掉review 的價值就在這里——把看不見的風(fēng)險提前暴露。6.3 可測試性Review 測試和 Review 代碼一樣重要代碼審查不能只看業(yè)務(wù)代碼測試代碼同樣在審查范圍內(nèi)。一個改動如果沒有對應(yīng)測試等于在提交一個承諾而這個承諾沒人驗證。我 review 測試時關(guān)注三點有沒有覆蓋核心狀態(tài)轉(zhuǎn)換、異常分支有沒有斷言、測試是在驗證行為還是在驗證實現(xiàn)細節(jié)。把大量 mock 堆起來的測試很容易變成“測試只驗證了 mock 本身”。依賴注入是提升可測試性最直接的手段與其在測試?yán)?patch 全局單例不如讓業(yè)務(wù)函數(shù)接受外部依賴作為參數(shù)提交者也更容易寫出自然覆蓋邊界的用例。6.4 演進成本臨時方案必須被記錄和認領(lǐng)技術(shù)債不是不能有但必須被記錄和認領(lǐng)。review 時看到# TODO、臨時補丁、異常分支爆炸我會問三個問題這個臨時方案會活多久誰負責(zé)在到期時把它換掉如果不換未來維護的人需要知道哪些隱性知識才能改對如果三個問題一個都答不上來那這次改動就不是在解決問題而是在把問題延后。代碼審查的最終目的不是讓當(dāng)前 MR 看起來完美而是讓整個系統(tǒng)在三個月、一年后依然有人敢改、改得動。7. 落地經(jīng)驗把檢查清單跑起來7.1 一張可以直接復(fù)制的最小檢查清單我把自己實際使用的檢查項壓縮成一張表可以直接放進團隊 MR 模板也可以作為自己提交前的自檢列表層次必查項不通過時的常見信號規(guī)范命名能自解釋函數(shù)短到可以一眼看完data、tmp、func2泛濫正確性空值、邊界、異常鏈、資源釋放都有處理except: pass、acquire和release隔著幾十行性能循環(huán)內(nèi)沒有 N1 查詢復(fù)雜度與數(shù)據(jù)量匹配循環(huán)里發(fā) HTTP、嵌套in查找安全用戶輸入不拼 SQL/路徑/命令密鑰不進日志f-string 拼 SQL、日志打印整個 request body架構(gòu)依賴單向、接口向后兼容、行為可測試循環(huán) import、改函數(shù)簽名沒搜調(diào)用點測試核心行為有覆蓋異常分支有斷言只測 happy path、mock 掉所有外部依賴依賴鎖文件存在依賴版本更新有記錄requirements 無版本、從 Git 直接拉取未固定 commit這張表不是一次 review 就要全部走完的而是按改動類型選擇。普通功能提交重點看規(guī)范、正確性和測試核心服務(wù)改動才需要把性能、安全和架構(gòu)全部過一遍。7.2 和 CI、Git 工作流結(jié)合人工 review 最怕的是把時間花在機器能判斷的問題上。靜態(tài)檢查、類型檢查、測試、依賴漏洞掃描都放進 CI讓提交者先在本地和流水線里解決掉“機器問題”人工 review 才有空間討論“人類問題”。在實際落地時我會建議團隊做幾件小事用統(tǒng)一的 pre-commit 配置鎖住提交格式CI 里不允許跳過測試大 MR 自動打標(biāo)簽提醒 reviewer 注意閱讀成本MR 模板里強制填寫“改動背景”和“測試說明”。這些措施的共性是把標(biāo)準(zhǔn)寫進流程而不是寄希望于每個人記得住。7.3 Reviewer 怎么和提交者溝通最后說溝通因為 review 的技術(shù)再熟練溝通失敗了也白搭。我的幾個原則是把“建議”和“必須改”明確分開避免所有意見混在一起讓提交者失去重點用提問代替斷言看到奇怪寫法先問“這里為什么這樣寫”而不是直接說“你寫錯了”能給出具體修改示例的盡量給出示例減少來回成本。我還有一個長期堅持的小習(xí)慣每次 review 至少寫一條 positive comment不是客套而是讓提交者知道什么樣的寫法應(yīng)該繼續(xù)保持。代碼審查如果只剩下挑錯大家會越來越怕提交代碼而一個好的 review 文化應(yīng)該讓提交者覺得這是在幫他把代碼變得更好。我現(xiàn)在每次打開一個 MR先按這個順序快速過一遍簽名、異常、資源、調(diào)用點、測試。剩下逐行細看的都是機器替我篩完之后值得人類判斷的東西。希望這份清單也能幫你少踩幾個坑。 Python代碼審查實戰(zhàn)清單：從規(guī)范到架構(gòu)的核心檢查點
把幾十個 Python 項目的 merge request 從頭到尾過一遍之后我對代碼審查的態(tài)度經(jīng)歷了一個反轉(zhuǎn)它不只是一道“檢查風(fēng)格是否符合規(guī)范”的關(guān)卡更是一個人把自己的判斷力注入項目的好機會。這篇文章不打算羅列教科書式的規(guī)范條目而是從實際 review 里沉淀下來的一套 Python 代碼審查實戰(zhàn)檢查清單——從最基本的編碼規(guī)范一路看到系統(tǒng)架構(gòu)每個檢查點都講清楚為什么要看、怎么看、以及我踩過什么樣的坑。適合剛接手 MR 評審的同學(xué)也適合想在提交前自己做一遍自檢的開發(fā)者。1. 代碼審查到底在看什么先分清四層目標(biāo)1.1 規(guī)范、正確性、設(shè)計與演進我最早做 review 的時候滿腦子都是“有沒有違反 PEP 8”“變量名是不是規(guī)范”結(jié)果把時間都花在了挑格式上真正有問題的邏輯反而被放過去了。后來我把 review 的目標(biāo)拆成四層每一層的審查方式和語氣都不一樣。規(guī)范層代碼能不能被人類順暢地讀懂命名是否誠實函數(shù)是否短到可以一眼看完。正確性層邊界條件、異常路徑、資源釋放、并發(fā)狀態(tài)在真實運行中會不會出問題。設(shè)計層模塊劃分是否合理依賴方向是否清晰新代碼是強化了既有結(jié)構(gòu)還是繞過了它。演進層這次改動三個月后會不會變成債接口是否向后兼容別人是否敢在這個基礎(chǔ)上繼續(xù)改。這四層的關(guān)系可以用下面這張表來理解層次關(guān)注點典型問題規(guī)范層可讀性、命名、結(jié)構(gòu)函數(shù)太長、命名誤導(dǎo)、邏輯藏在注釋里正確性邊界、異常、資源、競態(tài)空值崩潰、資源泄漏、重試不冪等設(shè)計層模塊劃分、依賴方向循環(huán) import、控制器里塞業(yè)務(wù)、工具模塊反向依賴演進層兼容性、擴展成本改簽名、動返回結(jié)構(gòu)、臨時補丁沒人認領(lǐng)每次 review 之前先問自己這次提交屬于哪一層大多數(shù)提交同時涉及多個層次但你的注意力分配不一樣。一個腳本實驗只要規(guī)范層過得去就行核心支付鏈路上的改動則必須把正確性和演進層都審到。換句話說代碼審查不是走流程而是先用“這層代碼會活多久”來判斷投入的精力。1.2 Diff 閱讀和全量閱讀的取舍很多人打開 MR 只看 diff逐行看新增代碼就結(jié)束了。我承認 diff 是高效手段但只靠 diff 會漏掉一類重要信息改動和周邊代碼的交互方式。我的做法是改動量在 200 行以內(nèi)把涉及的文件全量讀一遍超過 300 行先要求提交者把 MR 拆小拆不開的至少給出“核心改動”和“配套改動”的說明。全量閱讀能發(fā)現(xiàn)那種“新增函數(shù)看似通用其實和某個舊邏輯重復(fù)”的問題這類問題在 diff 里幾乎看不出來。另一個經(jīng)驗是如果提交者連 commit message 都寫不清“為什么改”那大概率連他自己都沒想清楚。review 從 commit message 開始能省掉后面大半的溝通成本。1.3 讀代碼的順序先接口再實現(xiàn)最后測試我推薦一套固定的閱讀順序先看公開函數(shù)的簽名和類型注解再讀實現(xiàn)里的異常分支最后看測試有沒有覆蓋關(guān)鍵路徑。先看簽名能快速判斷設(shè)計意圖比如get_user(user_id: int) - User | None比get_user(user_id)能傳遞的信息多得多看實現(xiàn)時重點盯“如果輸入非法會怎樣”而不是“正常輸入能不能跑通”最后看測試不是為了看覆蓋率數(shù)字而是確認提交者是否真的考慮過異常場景。這個順序能阻止你陷入局部細節(jié)。很多時候review 到一半突然發(fā)現(xiàn)接口設(shè)計本身就是錯的那后面幾十行格式問題已經(jīng)不值得再看了。2. 規(guī)范層PEP 8 之外真正影響可讀性的細節(jié)2.1 命名是低成本的架構(gòu)決策PEP 8 規(guī)定了命名風(fēng)格但沒規(guī)定命名質(zhì)量。我在 review 里看到最多的不是“命名不符合規(guī)范”而是“命名根本沒表達含義”。# 反例任何人都猜不到 process_data 具體做什么 def process_data(data): ... # 推薦函數(shù)名直接回答“做什么” def normalize_phone_number(raw: str) - str: ...data、item、temp、result這類變量名不是不能用而是出現(xiàn)頻率太高時讀代碼的人就得反復(fù)跳到定義處才能確認含義。每跳一次認知負荷就高一截。review 時看到新的變量名我會問自己一個新同事不看上下文能不能在 3 秒內(nèi)說出這個變量存的是什么如果不行這個命名就需要改。2.2 類型注解從文檔注釋到可執(zhí)行約束Python 是動態(tài)語言但團隊項目不應(yīng)該把“動態(tài)”當(dāng)作不做類型標(biāo)注的理由。類型注解最大的價值不是好看而是讓錯誤的可能性變窄同時給調(diào)用方一份不需要翻源碼的契約。# 不推薦 def get_user(user_id): return db.query(fSELECT * FROM users WHERE id {user_id}) # 推薦 def get_user(user_id: int) - User | None: return db.query_one(User, iduser_id)review 時看到# type: ignore我不會直接放過去而是按“這里藏著一個解釋不清的問題”來處理。它可能是第三方庫的 isue也可能是類型設(shè)計本身有問題。對于 Python 3.10 以下的團隊int | None要寫成Optional[int]這點在 review 時要根據(jù)項目版本保持一致。注解也不是越多越好。內(nèi)部函數(shù)和私有方法可以少注解公開接口必須完整尤其是返回類型。返回類型缺失調(diào)用方拿到什么都只能靠猜這是規(guī)范層里最隱蔽的成本。2.3 函數(shù)長度與認知負荷“一個函數(shù)最好不超過 50 行”這類建議太絕對我更喜歡用“一眼定律”打開函數(shù)視線掃一遍能不能看出它總共在干幾件事。如果一個函數(shù)做到了三件事解析輸入、調(diào)外部服務(wù)、寫緩存那么即使只有 30 行也應(yīng)該拆。相反一個線性執(zhí)行的純計算函數(shù)長一點也沒太大問題因為它的邏輯是連續(xù)的。真正危險的是多層縮進、循環(huán)套條件、條件里再調(diào)函數(shù)這種結(jié)構(gòu)讓每一行的狀態(tài)都依賴前面的狀態(tài)讀起來像是在做思維迷宮。review 時看到超過 50 行且多層嵌套的函數(shù)第一反應(yīng)不是“給我拆成更短”而是“它同時在干幾件事”。讓提交者把職責(zé)拆開往往比單純要求縮短更有用。2.4 自動化工具能做的別讓人眼做格式、未使用的變量、import 順序、明顯的語法問題這些全部應(yīng)該交給工具。我的人工注意力只留給工具判斷不了的問題語義、邊界、依賴、演進。一個可復(fù)制的組合是用自動格式化工具統(tǒng)一代碼風(fēng)格用靜態(tài)檢查工具掃常見問題用類型檢查工具驗證注解最后用統(tǒng)一配置的 pre-commit 在提交前攔截。團隊里最想避免的是每個開發(fā)者本地配置不同導(dǎo)致 MR 里全是格式噪音真正的邏輯問題反而被淹沒。檢查項交給工具必須人眼縮進、行寬、import 排序是否未使用變量、未定義名稱是否類型不匹配是否命名是否誤導(dǎo)否是注釋是否與代碼同步過時否是函數(shù)職責(zé)是否單一否是3. 正確性與異常路徑最容易放走的隱蔽 Bug3.1 空值、邊界和“看起來不可能”的輸入Review 里最常漏掉的 bug往往出現(xiàn)在“正常路徑?jīng)]有問題極端輸入直接崩潰”的地方。列表分片、字典取值、鏈?zhǔn)秸{(diào)用都是重災(zāi)區(qū)。# 看起來沒問題但 items 為空時會發(fā)生什么 batch_size len(items) // 3 for chunk in [items[i:i batch_size] for i in range(0, len(items), batch_size)]: process(chunk)當(dāng)items為空時batch_size是 0range(0, 0, 0)會直接拋ValueError。這個例子說明review 時看到列表運算至少要在心里做一遍“空列表、單元素列表、n 不能被整除”的推演。我自己在 review 訂單回調(diào)邏輯時遇到過類似問題生產(chǎn)環(huán)境里數(shù)據(jù)量偶爾為 0測試數(shù)據(jù)從沒覆蓋上線后第一分鐘就報警。字典的鏈?zhǔn)饺≈狄彩峭瑯拥倪壿媟esult data.get(key, {}).get(value, 0)如果data[key]存在但值是None這一行照樣拋AttributeError。review 時看到鏈?zhǔn)饺≈叼B(yǎng)成習(xí)慣問一句中間節(jié)點可能為 None 嗎3.2 異常粒度什么時候 catch什么時候讓它拋出去except Exception是 Python 項目里最讓人頭疼的寫法之一。它看起來安全實際上把所有異常都吞掉了包括代碼里真正的 bug。# 反例失敗時完全不知道發(fā)生了什么 try: response requests.post(url, jsonpayload, timeout5) except Exception: pass # 推薦區(qū)分異常類型并保留原始上下文 try: response requests.post(url, jsonpayload, timeout5) except requests.Timeout as exc: raise ExternalServiceTimeout(payment service timeout) from exc except requests.RequestException as exc: raise ExternalServiceError(payment service unavailable) from excraise ... from exc不是形式主義它能把底層異常鏈完整保留排查問題時少走很多彎路。review 時看到裸except: pass基本可以直接打回。如果確實只需要兜底日志至少要用logger.exception(...)記錄完整堆棧而不是讓異常靜默消失。3.3 資源與上下文with 之外的隱藏狀態(tài)Python 程序員都知道with open(...)可以安全處理文件但資源管理的坑遠不止文件。數(shù)據(jù)庫連接、線程鎖、臨時目錄、信號量、緩存客戶端都需要明確的生命周期。# 不推薦acquire 和 release 隔著 20 行中間任何 return 都會漏釋放 lock.acquire() try: ... finally: lock.release() # 推薦用上下文管理器收斂生命周期 with lock: ...review 時看到acquire()和release()相距很遠必須警惕。這類代碼一旦中間出現(xiàn)提前 return 或者新異常鎖就永遠放不掉了。更好的方式是把資源生命周期收進with語句或顯式的上下文管理器里讓“進入和退出”成對出現(xiàn)。另外一個容易被忽略的資源是緩存和連接池。比如每請求創(chuàng)建新的數(shù)據(jù)庫連接而不是復(fù)用連接池這在低并發(fā)時沒事流量上來就直接打滿連接數(shù)。3.4 按“發(fā)生中斷后怎么辦”來讀正確性 review 的最高標(biāo)準(zhǔn)不是正常流程是否能跑通而是每一步“發(fā)生中斷后系統(tǒng)是否還能自洽”。舉個例子處理支付回調(diào)時是先改訂單狀態(tài)還是先記賬如果改完狀態(tài)、記賬之前進程崩了重試會發(fā)生什么訂單會不會被重復(fù)記賬如果調(diào)用外部接口超時重試邏輯會不會把同樣的請求發(fā)兩次這類問題在 happy path 里看不見但生產(chǎn)環(huán)境里幾乎天天發(fā)生。review 時看到寫數(shù)據(jù)庫、發(fā)消息、調(diào)外部服務(wù)的代碼我會要求提交者說明“重試一次會發(fā)生什么”。如果對方答不上來說明這個改動還沒想清楚。冪等鍵、去重表、狀態(tài)機限制都是這類問題的常見解法但更重要的是先把問題問出來。4. 性能與并發(fā)Review 時如何定位潛在瓶頸4.1 復(fù)雜度預(yù)判先看循環(huán)嵌套和數(shù)據(jù)規(guī)模性能 review 不是靠猜而是靠量級判斷。看到嵌套循環(huán)時我會先確認數(shù)據(jù)規(guī)模外層 10 條、內(nèi)層 1000 條和內(nèi)外都是 10 萬條處理方式完全不同。# O(n*m)每個元素都在列表里做一次線性查找 for user_id in user_ids: if user_id in all_user_ids: ... # O(nm)先用集合建立索引 known_ids set(all_user_ids) for user_id in user_ids: if user_id in known_ids: ...把列表in查找換成集合或字典是 Python 項目里性價比最高的性能改進。但 review 時也要克制如果數(shù)據(jù)規(guī)模只有幾百這種優(yōu)化對用戶毫無感知反而讓代碼多幾行。性能問題的優(yōu)先級應(yīng)該由“實際量級”決定而不是由“聽起來很慢”決定。4.2 數(shù)據(jù)庫查詢和 IO 重災(zāi)區(qū)會延續(xù)Python Web 服務(wù)里最常見的性能問題是 N1 查詢外層循環(huán)查詢主記錄循環(huán)里再一次次查關(guān)聯(lián)數(shù)據(jù)。review 時看到循環(huán)里執(zhí)行數(shù)據(jù)庫查詢直接標(biāo)記。# N1 查詢 for order in orders: items.append(get_items_for_order(order.id)) # 批量查詢 items get_items_for_orders([o.id for o in orders])同樣的道理適用于循環(huán)里發(fā) HTTP 請求、循環(huán)里寫文件、循環(huán)里調(diào)第三方 SDK。每一次 IO 都有網(wǎng)絡(luò)延遲和連接開銷批量合并往往能帶來數(shù)量級的提升。review 時還要看事務(wù)邊界長事務(wù)里做外部調(diào)用會讓數(shù)據(jù)庫連接長時間占用連接池一旦耗盡整個服務(wù)都會受影響。4.3 多線程與異步可重入、競態(tài)與阻塞多線程代碼在 review 里最難發(fā)現(xiàn)的是競態(tài)問題。比如一個看似簡單的計數(shù)器counter 0 def worker(): global counter for _ in range(100): counter 1counter 1不是原子操作線程交替執(zhí)行時會出現(xiàn)“讀-算-寫”中間被切換的情況。review 時看到共享狀態(tài)被多線程讀寫要么加鎖要么用queue或concurrent.futures的線程安全結(jié)構(gòu)。異步代碼也有自己的坑。在async函數(shù)里調(diào)用同步阻塞 IO會直接卡住整個事件循環(huán)導(dǎo)致其他請求一起變慢。review 時看到await函數(shù)里混著time.sleep或同步requests應(yīng)該提示改成異步版本或者放到線程池里執(zhí)行。4.4 沒有 profile 不下定論性能 review 最容易犯的錯誤是憑直覺要求“優(yōu)化”。有一次我 review 一個數(shù)據(jù)校驗功能建議把某處的list換成set理由是從復(fù)雜度上看更快。后來提交者實際測試數(shù)據(jù)量只有幾百條改動前后耗時差異在誤差范圍內(nèi)反而因為引入集合的哈希約束代碼可讀性變差了。從那以后我給自己定了一條紀(jì)律review 時可以指出明顯的復(fù)雜度問題但涉及微優(yōu)化必須給出可驗證的收益。通常先問“這里的數(shù)據(jù)量級是多少”“有沒有壓測數(shù)據(jù)”再決定要不要動。5. 安全與依賴代碼審查中的風(fēng)險掃描5.1 注入與路徑操作把不可信輸入當(dāng)敵人Python 里最容易出現(xiàn)的注入是 SQL 注入和命令注入。review 時看到字符串拼接 SQL無論提交者怎么解釋都先假定是有問題的。# 反例用戶輸入直接拼進 SQL query fSELECT * FROM users WHERE name {name} # 正例參數(shù)化查詢 query SELECT * FROM users WHERE name ?文件路徑也一樣。用戶傳入的文件名如果包含../直接拼進os.path.join(base_dir, filename)就能越界讀到其他目錄。review 時看到用用戶輸入構(gòu)造路徑至少要用Path.resolve()校驗最終路徑是否仍然位于允許的目錄內(nèi)。命令注入則藏在subprocess.run(fls {path}, shellTrue)這類寫法里。能不用shellTrue就不用改用參數(shù)列表傳參是更安全的方式。5.2 敏感信息密鑰、Token 與日志泄露很多密鑰事故不是發(fā)生在配置中心而是發(fā)生在日志里。review 時看到logger.info(request failed: %s, request_body)要考慮request_body里是否包含密碼、Token、手機號一類信息。一個很常見的場景是開發(fā)環(huán)境調(diào)試時為了方便把整個請求體打到日志里上線的時候忘了刪。我的檢查習(xí)慣是全文搜索幾個關(guān)鍵詞password、token、secret、api_key看它們有沒有出現(xiàn)在日志輸出、錯誤消息、URL 參數(shù)和數(shù)據(jù)庫字段里。如果確實需要記錄請求體至少要做脫敏處理比如把密碼字段替換成***。5.3 依賴鎖定與第三方風(fēng)險requirements.txt里寫package1.0等于把版本命運交給未來。review 時看到依賴沒有鎖定版本或者鎖定版本但不鎖定傳遞依賴我會要求改成固定版本加鎖文件的方式保證任何人在任何時候拉代碼裝出來的環(huán)境是一致的。另一個常被忽略的問題是直接從一個 Git 倉庫pip install git...安裝依賴。這不一定是錯的但必須固定到具體 commit 或者 tag否則第三方倉庫的每次推送都會改變你的構(gòu)建結(jié)果。定期掃描已安裝依賴的已知漏洞也應(yīng)該進入團隊的例行任務(wù)。5.4 服務(wù)端校驗與權(quán)限前端校驗只是用戶體驗服務(wù)端校驗才是安全邊界。review 時看到新接口沒有校驗必填字段、長度、格式直接打回。尤其要注意接口是否真的做了權(quán)限判斷而不是只依賴網(wǎng)關(guān)過濾。同事曾經(jīng)在網(wǎng)關(guān)層做了 auth 校驗業(yè)務(wù)層就默認所有請求都合法直到有人發(fā)現(xiàn)一個內(nèi)部接口可以被普通用戶直接調(diào)用改了好幾天才補完。6. 架構(gòu)層面這次改動是否在給系統(tǒng)挖坑6.1 依賴方向與分層讓數(shù)據(jù)流單向流動代碼審查要看的最后一個大層次是這次改動對整體結(jié)構(gòu)的影響。我最常遇到的架構(gòu)問題是循環(huán) import 和分層混亂。一個典型的信號是視圖函數(shù)里寫業(yè)務(wù)邏輯、業(yè)務(wù)函數(shù)里拼 SQL、而工具模塊反過來 import 業(yè)務(wù)模塊。數(shù)據(jù)流一旦開始繞圈任何改動都有可能踩到隱藏的耦合。review 時如果發(fā)現(xiàn)自己畫不出“這次改動從哪里來、要往哪里去”的依賴關(guān)系就應(yīng)該打回去要求重構(gòu)。推薦的做法是讓依賴方向保持單向控制層調(diào)服務(wù)層服務(wù)層調(diào)數(shù)據(jù)訪問層工具模塊只提供無業(yè)務(wù)含義的通用能力不反向依賴業(yè)務(wù)。分層不是教條而是讓未來改任何一層的成本都可控。6.2 接口設(shè)計與向后兼容改函數(shù)簽名、改字典 key、改返回類型意味著所有調(diào)用方都要跟著變。review 時看到這類改動第一件事是在倉庫里搜索所有調(diào)用點確認影響范圍。# 原來返回 dict調(diào)用方用 config[timeout] 獲取值 def get_config() - dict: ... # 改成返回 dataclass調(diào)用方原來的寫法全部失效 def get_config() - AppConfig: ...如果這是內(nèi)部接口可以保留舊函數(shù)并加 deprecated 警告先讓調(diào)用方遷移再在下一個版本移除。如果是對外 HTTP API優(yōu)先考慮新增版本而不是原地修改。一次“順手”的接口變更可能讓下游系統(tǒng)凌晨炸掉review 的價值就在這里——把看不見的風(fēng)險提前暴露。6.3 可測試性Review 測試和 Review 代碼一樣重要代碼審查不能只看業(yè)務(wù)代碼測試代碼同樣在審查范圍內(nèi)。一個改動如果沒有對應(yīng)測試等于在提交一個承諾而這個承諾沒人驗證。我 review 測試時關(guān)注三點有沒有覆蓋核心狀態(tài)轉(zhuǎn)換、異常分支有沒有斷言、測試是在驗證行為還是在驗證實現(xiàn)細節(jié)。把大量 mock 堆起來的測試很容易變成“測試只驗證了 mock 本身”。依賴注入是提升可測試性最直接的手段與其在測試?yán)?patch 全局單例不如讓業(yè)務(wù)函數(shù)接受外部依賴作為參數(shù)提交者也更容易寫出自然覆蓋邊界的用例。6.4 演進成本臨時方案必須被記錄和認領(lǐng)技術(shù)債不是不能有但必須被記錄和認領(lǐng)。review 時看到# TODO、臨時補丁、異常分支爆炸我會問三個問題這個臨時方案會活多久誰負責(zé)在到期時把它換掉如果不換未來維護的人需要知道哪些隱性知識才能改對如果三個問題一個都答不上來那這次改動就不是在解決問題而是在把問題延后。代碼審查的最終目的不是讓當(dāng)前 MR 看起來完美而是讓整個系統(tǒng)在三個月、一年后依然有人敢改、改得動。7. 落地經(jīng)驗把檢查清單跑起來7.1 一張可以直接復(fù)制的最小檢查清單我把自己實際使用的檢查項壓縮成一張表可以直接放進團隊 MR 模板也可以作為自己提交前的自檢列表層次必查項不通過時的常見信號規(guī)范命名能自解釋函數(shù)短到可以一眼看完data、tmp、func2泛濫正確性空值、邊界、異常鏈、資源釋放都有處理except: pass、acquire和release隔著幾十行性能循環(huán)內(nèi)沒有 N1 查詢復(fù)雜度與數(shù)據(jù)量匹配循環(huán)里發(fā) HTTP、嵌套in查找安全用戶輸入不拼 SQL/路徑/命令密鑰不進日志f-string 拼 SQL、日志打印整個 request body架構(gòu)依賴單向、接口向后兼容、行為可測試循環(huán) import、改函數(shù)簽名沒搜調(diào)用點測試核心行為有覆蓋異常分支有斷言只測 happy path、mock 掉所有外部依賴依賴鎖文件存在依賴版本更新有記錄requirements 無版本、從 Git 直接拉取未固定 commit這張表不是一次 review 就要全部走完的而是按改動類型選擇。普通功能提交重點看規(guī)范、正確性和測試核心服務(wù)改動才需要把性能、安全和架構(gòu)全部過一遍。7.2 和 CI、Git 工作流結(jié)合人工 review 最怕的是把時間花在機器能判斷的問題上。靜態(tài)檢查、類型檢查、測試、依賴漏洞掃描都放進 CI讓提交者先在本地和流水線里解決掉“機器問題”人工 review 才有空間討論“人類問題”。在實際落地時我會建議團隊做幾件小事用統(tǒng)一的 pre-commit 配置鎖住提交格式CI 里不允許跳過測試大 MR 自動打標(biāo)簽提醒 reviewer 注意閱讀成本MR 模板里強制填寫“改動背景”和“測試說明”。這些措施的共性是把標(biāo)準(zhǔn)寫進流程而不是寄希望于每個人記得住。7.3 Reviewer 怎么和提交者溝通最后說溝通因為 review 的技術(shù)再熟練溝通失敗了也白搭。我的幾個原則是把“建議”和“必須改”明確分開避免所有意見混在一起讓提交者失去重點用提問代替斷言看到奇怪寫法先問“這里為什么這樣寫”而不是直接說“你寫錯了”能給出具體修改示例的盡量給出示例減少來回成本。我還有一個長期堅持的小習(xí)慣每次 review 至少寫一條 positive comment不是客套而是讓提交者知道什么樣的寫法應(yīng)該繼續(xù)保持。代碼審查如果只剩下挑錯大家會越來越怕提交代碼而一個好的 review 文化應(yīng)該讓提交者覺得這是在幫他把代碼變得更好。我現(xiàn)在每次打開一個 MR先按這個順序快速過一遍簽名、異常、資源、調(diào)用點、測試。剩下逐行細看的都是機器替我篩完之后值得人類判斷的東西。希望這份清單也能幫你少踩幾個坑。