Revolutionizing Code Review: The Power of SonarQube

Revolutionizing Code Review: The Power of SonarQube

Code Review לא אמור לקחת חמישה ימים. לא כי מישהו קורא diff חמישה ימים ברצף, אלא כי הקוד עובר פינג פונג: הערה, תיקון, עוד הערה, מפתח שלא מחובר עכשיו, ריוויור שפנוי רק מחר. ובין כל אלה מסתתרות הערות על semicolon, duplication או naming שאפשר היה לתפוס לפני שה-PR הגיע לאדם אחר.

בהרצאה של אלעד בטיט, Staff Software Engineer ב-AppsFlyer, הוא מציג את SonarQube ככלי שמנסה להוציא את העבודה החזרתית הזאת מ-Code Review. הוא לא מחליף שיקול דעת אנושי. הוא עושה משהו אחר: בודק את הקוד החדש מול חוקים ומדדים, ומאפשר לריוויור להקדיש זמן למה שבאמת דורש הבנת מערכת - business logic, flow ו-tradeoffs.

הבעיה היא לא רק הזמן, אלא סוג השיחה

אלעד מצטט מחקר שבחן מיליון pull requests ומראה שהחלק הארוך בתהליך הוא הריוויו. הסיבה אינה בהכרח גודל השינוי. שעות העבודה מפוזרות, השיחה נמרחת, וקוד הוא דבר אישי: מפתח שולח יצירה שלו ומקבל עליה הערות. לפעמים ההערה נכונה, אבל הדרך עד שמגיעים אליה הופכת את כל התהליך לכבד.

המטרה אינה לבטל Code Review. כל שינוי עדיין צריך מישהו שיבדוק האם הלוגיקה נכונה, האם המקרה העסקי טופל, ומה יקרה כששירות אחר מחזיר משהו לא צפוי. אבל בדיקות סגנון, בעיות חוזרות וסימנים של קוד מסוכן לא צריכות להישען רק על ערנות של ריוויור.

מה SonarQube בודק

SonarQube עובד בכמה רמות. למפתח הוא נותן feedback ב-IDE או אחרי commit על הקוד החדש. לצוות הוא נותן תמונה של הפרויקט, כולל legacy code וחוב טכני. ברמת הארגון הוא מציג dashboards על quality, security, coverage ו-maintainability, כך שאפשר להגדיר סטנדרטים אחידים גם לצוותים בשפות שונות.

ברמת הבדיקה עצמה הוא מזהה code smells, באגים פוטנציאליים, duplication ומורכבות. הוא יודע להצביע על פונקציות מסועפות וקשות לתחזוקה, ולא רק לספור שורות. הוא גם בודק coverage מתוך הדוח שכלי הטסטים מפיק. בתחום האבטחה אלעד מדגים credentials שנכנסו לקוד, ומציין יכולות לזיהוי SQL injection ו-cross-site scripting במהדורות שתומכות בכך.

זה המקום שבו הכלי משלים ESLint. ESLint כבר נותן הרבה בעולם JavaScript. SonarQube מוסיף איכות, security, complexitiy, duplication ומדדים שמחברים את הקובץ הבודד לתמונה רחבה יותר. לא כל rule מתאים לכל צוות, ולכן צריך לכוון אותו ולא פשוט להפעיל הכול ולקוות לטוב.

Quality Gate הופך איכות להחלטה מפורשת

הסריקה יכולה לרוץ בשלושה מקומות. Plugin ב-IDE תופס בעיה מוקדם. ב-CI היא רצה שוב על השינוי. וב-GitHub או GitLab הריוויור מקבל report על ה-merge request. ה-report נבדק מול Quality Gate - סף שהצוות מגדיר בעצמו.

אפשר לקבוע כמה coverage נדרש לקוד חדש, כמה duplication מותר, ואיזה ציון maintainability, reliability או security חייב לעבור. יש ערך בסטנדרט, כי הוא הופך איכות מציפייה עמומה לכלל שאפשר לראות ולמדוד. אבל ה-gate צריך להיות מציאותי. רף שאי אפשר לעבור רק ילמד את הצוות לחפש bypass.

אלעד מראה בדמו שאפשר להתחיל מקומית: להרים SonarQube ב-Docker, לפתוח localhost:9000, ליצור project ו-token, ולהריץ Sonar Scanner מתוך פרויקט. בחלק מהשפות והמהדורות צריך עוד configuration, ויש גם יכולות שאינן זמינות במהדורה החינמית. חשוב לדעת מה באמת נסרק, ולא להניח שכל security issue ייתפס אוטומטית.

דמו קטן חושף בעיות אמיתיות

בדוגמה שלו יש פרויקט React ו-Express שאלעד שתל בו בעיות בכוונה. אחת מהן היא סיסמה בתוך חיבור ל-MySQL. SonarQube מציג את בעיית האבטחה ומסביר שה-credential לא אמור להיות ב-Git. בעיה אחרת היא קריאה ל-setState בתוך גוף קומפוננטת React, שמובילה ל-render אינסופי. גם אותה הדשבורד מזהה כ-bug ומצביע על המקום והדרך לתקן.

זו לא רק רשימת שגיאות. אפשר להקצות בעיה לאדם, להגדיר priority, ולעבור מהדשבורד לקובץ המדויק. במקביל אפשר לראות אזורים עם coverage נמוך וחוב טכני גבוה. עבור ריוויור, המשמעות היא שדוח אחד מציף את הבדיקות המכאניות עוד לפני שצללו ל-diff.

למי שרוצה להעמיק באיך בודקים מעבר ליחידה אחת, ההרצאה על בדיקות API עוסקת בסיכון שבין שירותים. SonarQube אינו מחליף את הבדיקות האלה. הוא שומר על גבול אחר: איכות הקוד שמכניסים לפני שהמערכת בכלל מגיעה לתרחישי אינטגרציה.

אל תנסו לתקן את כל ה-legacy ביום הראשון

זה החשש הטבעי: מפעילים כלי על repository בן חמש שנים, ומקבלים אלפי warnings. מי אמור לתקן את כל זה? אלעד מציג את ההתמקדות ב-new code כדרך לצאת מהמלכודת. הבעיות הקיימות נשארות גלויות, אבל השאלה היומיומית היא האם הקוד שנוסף עכשיו עומד ברף.

הגישה הזאת יוצרת אחריות בלי לעצור פיתוח. כל שינוי חדש לא מכניס עוד חוב, ובהדרגה המערכת משתפרת. זה גם מפחית noise. אם הכלי רק מייצר התראות שאף אחד לא קורא, הוא הופך לרעש. אם הוא מחובר ל-PR ולכללים שהצוות באמת מוכן לאכוף, הוא הופך לחלק מתהליך העבודה.

זה גם משנה את ההתנהגות לפני ה-PR. אלעד מספר שמפתח שרואה coverage או duplication בדוח לפני שהוא שולח לריוויו כבר מתקן חלק מהדברים בעצמו. הריוויור מתחיל מאותה תמונה: הוא יכול לשאול אם התייחסו לבעיה שהכלי מצא, ואז להקדיש את הקריאה להחלטות ולא לרדיפה אחר פרטים בסיסיים. קשה למדוד במדויק כמה זמן זה חוסך, אבל סבבי “תבדוק שוב” מתקצרים כשההערות המכאניות לא מחכות לאדם הבא בתהליך.

יש גם מחיר. לא כל שפה נתמכת באותה רמה, דוח coverage אינו מובנה בתוך SonarQube אלא מגיע מכלי הטסטים, וחיבור מלא ל-CI דורש לקרוא documentation ולהגדיר נכון את הפרויקט. אלו לא סיבות לוותר. אלו דברים שצריך להבין מראש, לכוון לפי ה-stack, ולבדוק מול ערך אמיתי לצוות במקום להכניס עוד מוצר כי כולם משתמשים בו.

השורה התחתונה

SonarQube אינו רובוט שמחליף ריוויור. הוא מוציא מהריוויו את החלקים שהרובוט טוב בהם, כדי שהאנשים יבדקו את מה שמסוכן באמת.

• הפעילו בדיקות מוקדם ב-IDE ושוב ב-CI.
• הגדירו Quality Gate שמתאים לצוות ולסוג הפרויקט.
• תנו לכלי לבדוק duplication, complexity, coverage וחלק מבעיות האבטחה.
• השאירו לריוויור את הלוגיקה העסקית ואת התכנון.
• התחילו מ-new code כדי לא להיתקע מול חוב legacy אינסופי.
• כוונו את החוקים. כלי שמייצר noise לא מגן על איכות.

כשההערות האוטומטיות מגיעות לפני ה-PR, הדיון האנושי יכול סוף סוף לעבור מה-semicolon לשאלה אם הפיצ’ר באמת נכון.