Code-analyseregel formatteren met Android Lint (deel 1/2)

Voor Kotlin en Java tegelijk

Balázs Ruda
Balázs Ruda

Follow

9 sep, 2019 – 9 min read

TL;DR

In dit eerste deel van het artikel wil ik in de eerste plaats laten zien dat het de moeite waard is om een aangepaste code-analyseregel te schrijven om codeconventies af te dwingen. Ten tweede wil ik laten zien dat naast Checkstyle en ktlint, Android Lint ook moet worden overwogen bij het maken van een opmaak-gerelateerde code analyse regel, ondanks dat dat niet zijn basis doel is. Ik zal uitleggen wat in mijn geval het belangrijkste voordeel is ten opzichte van de andere tools, en de innerlijke werking beschrijven die dit voordeel oplevert. Ter inspiratie zal ik ook de stappen laten zien die ik heb genomen om mijn formatteer regel te laten werken met Android Lint.

Motivatie

Ik raak gefrustreerd als ik formatteer-gerelateerd commentaar krijg op mijn samenvoeg verzoeken of als ik dergelijk commentaar moet toevoegen aan die van anderen. Soms moeten we takken veranderen om een enkele formatteringsfout in onze MR op te lossen, wat behoorlijk demotiverend is. Daarom ben ik gaan nadenken over manieren om dit kleine (maar belangrijke) probleem automatisch op te lossen, zodat we ons tijdens code-reviews op complexere zaken kunnen richten.

We gebruiken verschillende lint-tools, zoals ktlint of Checkstyle, voor het afdwingen van globale opmaakconventies (volgorde van variabelen, regellengte, etc.), maar deze bevatten standaard niet bepaalde bedrijfsbrede regels. Voor die gevallen ondersteunen de hierboven genoemde tools ook het toevoegen van aangepaste regels. Ik besloot er een te schrijven voor de meest voorkomende opmaakfouten die we maken:

Missing empty line around “block statements” issue

We vereisen altijd dubbele regeleindes voor en na “block statements”, wat if, switch (wanneer in Kotlin), for, try, of while blokken kunnen zijn, tenzij ze precies aan het begin of het einde van een methode of een ander blok staan. Wij geloven dat dit onze code leesbaarder maakt, daarom laten we tijdens de code review altijd een opmerking achter als iemand deze regel overtreedt (en als het ons toevallig opvalt).

Wat is de beste tool voor dit probleem

We kunnen zeggen dat ktlint voor Kotlin en Checkstyle voor Java de belangrijkste tools voor het analyseren van opmaakcode zijn, maar nu kies ik toch een andere tool: Android Lint (niet alleen voor Android projecten), omdat het ook ondersteunt het schrijven van aangepaste regels die kunnen worden toegepast op Kotlin en Java op hetzelfde moment. Het lijkt een logischere keuze, omdat we beide talen gebruiken in onze Android projecten, en ik wilde niet twee keer dezelfde regel schrijven of ze tegelijkertijd onderhouden. Om nog maar te zwijgen over de integratie van de regel, die ook twee keer zou moeten gebeuren.

Waarom Java überhaupt nog belangrijk is

Als Android-ontwikkelaar moet ik code schrijven in zowel Java als Kotlin. We kunnen echter zeggen dat het de tijd niet meer waard is om ons op Java te richten, want nu Kotlin in opkomst is, gebruiken we het meer en meer in plaats van Java in onze codebase. Aan de andere kant denk ik dat de overgang niet zo snel gaat in grote projecten die al in productie zijn. Dus, omdat we ook in Java netjes opgemaakte code willen zien, is het belangrijk om deze regel voor beide talen te hebben.

Hoe schrijf je aangepaste lint regels

Er zijn veel tutorials en artikelen over het schrijven van aangepaste code analyse regels, dus ik zal er in deze post niet te veel over uitweiden. Maar als je geïnteresseerd bent, zijn hier een paar links die ik je kan aanraden voor de verschillende tools: Naar Checkstyle, de officiële doc, naar ktlint en Android Lint, Niklas Baudy’s geweldige medium artikelen.

Exemplaar voor AST (bron: Wikipedia over Abstract Syntax Tree)

Hoe Android Lint en andere statische code-analysetools werken

In eerste instantie werd Android Lint gemaakt voor het vinden van voornamelijk Android-specifieke problemen in Java-code. Om de Java-code te analyseren, maakt Android Lint een Java-specifieke Abstract Syntax Tree (of gewoon AST, leer meer op Wikipedia), dat is een boom-weergave van de broncode. Andere statische code analyse tools gebruiken ook een taalspecifieke AST voor analyse;
Checkstyle: Java-, ktlint: Kotlin-, detekt: Kotlin-specifiek. De tools doorkruisen deze AST en vinden fouten door de knooppunten en hun attributen te controleren.

Hoe Android Lint twee talen tegelijk kan controleren

Het verschil tussen Android Lint en andere tools is dat toen Kotlin ook een ondersteunde taal werd voor Android, Android Lint ook Kotlin is gaan ondersteunen met betrekking tot de regels die we al hadden voor Java. Hiervoor introduceerden ze de zogenaamde Universal Abstract Syntax Tree (ontwikkeld door JetBrains) die een boom representatie van de code biedt die kan worden toegepast voor zowel Kotlin als Java talen op hetzelfde moment, dus het is op een hoger abstractieniveau dan een taalspecifieke AST. Voor meer informatie raad ik dit praatgedeelte van Tor Norbye aan – de maker en onderhouder van Android Lint.

Zowel UAST als AST geven op hoog niveau details over de broncode. Ze bevatten geen informatie over whitespaces of accolades, maar dat is meestal genoeg voor de Android specifieke regels. Zie hieronder een UAST voorbeeld:

Door de asRecursiveLogString()-methode aan te roepen op een bepaalde UAST-node in de Android Lint API, kunnen we de UAST afdrukken.

De UAST-bibliotheek bevat alle taalspecifieke expressies, maar biedt ook gemeenschappelijke interfaces voor deze expressies. Voorbeeld voor de uitdrukking if:

UIfExpression gemeenschappelijke interface wordt geïmplementeerd door de JavaUIfExpression, JavaUTernaryExpression, en KotlinUIfExpression.

PSI Tree (Program Structure Interface Tree)

De PSI-boom is gebouwd op de UAST in het geval van Android Lint (in het geval van andere tools is deze gebouwd op de taalspecifieke AST), en dit is de boom die meer details bevat over de structuur van de code, zoals witruimtes, accolades en soortgelijke elementen.

In Android Lint zijn PSI Expressions opgenomen in de implementatie van de UAST Expressions, die de knooppunten van de UAST zijn. Bv. org.jetbrains.kotlin.psi.KtIfExpression kan worden benaderd vanuit KotlinUIfExpression.

Er is zelfs een handige intelliJ plugin: PsiViewer die het debuggen van PSI tree-based code analyse regels kan vergemakkelijken. Zie het voorbeeld hieronder met dezelfde code snippet, waar we kunnen zien dat de twee talen verschillende taalspecifieke tokens in de bomen hebben:

PSI-boomrepresentatie van een methode in Java en Kotlin

Gebruik van zowel UAST als PSI Tree

Ik heb zowel UAST als PSI Tree nodig om mijn opmaakregel te maken, omdat UAST geweldig is om te filteren en nodes te bezoeken waarin ik geïnteresseerd ben voor beide talen tegelijk, maar zoals ik al zei geeft het geen informatie over opmaak, bijv. witruimte informatie die essentieel is voor mij. UAST richt zich eerder op een hoger abstractieniveau, dus primair is het niet voor het maken van opmaakregels.

Sinds de Gradle plugin 3.4 en de gerelateerde Android Lint versie 26.4.0, kunnen we de PSI tree representatie van elke node en zijn omgeving krijgen, niet alleen in Java, maar ook in Kotlin. Dit maakt het mogelijk om Android Lint te gebruiken voor mijn opmaakregel.

Hoe de regel is geïmplementeerd

Eerst moet ik mijn issue aanmaken, waarbij ik de scope instel op JAVA_FILE, wat op dit moment zowel Java als Kotlin betekent in Android Lint. (Hier is waar we kunnen instellen andere soorten bestanden ook, zoals XML of Gradle-bestanden, omdat Android Lint kan controleren ze ook.)

Initialisatie van een aangepaste Issue-klasse

Dan, in mijn detector-klasse, som ik de knooppunten op waarin ik geïnteresseerd ben in de functie getApplicableUastTypes, zodat Android Lint weet dat het de gerelateerde overridden bezoekmethoden moet aanroepen, zoals visitForEachExpression. In elke bezoek methode, roep ik gewoon mijn checker methode aan.

Hoe vertel ik Android Lint welke knooppunten we willen controleren

In mijn checker methode, beschrijft de forward parameter of ik de regeleinde voor of na het “blok statement” wil controleren. In de body van de methode onderzoek ik het aantal witregels rond de blokverklaring die ik bezoek. Daarvoor doe ik drie hoofdstappen:

  • Eerst controleer ik met de methode firstBlockLevelNode wat het eerste blokniveau-knooppunt is, waar omheen ik de witregels wil controleren, want als ik een if-blok gebruik voor het toewijzen van een waarde aan een variabele, zoals dit,
“Blokstatement” in waardetoekenning

Lint zou de witruimte net voor het if-sleutelwoord onderzoeken, maar ik ben geïnteresseerd in de witruimte voor het val-sleutelwoord. In dit geval is de eerste instructie op blokniveau dus de waardetoekenning die mijn if-instructie omhult.

  • Ten tweede controleer ik in de firstRelevantWhiteSpaceNode-methode wat de eerste relevante witruimte-node is, waar we de regeleinden moeten tellen. Soms is er geen relevante witruimte om te controleren, want als mijn blok aan het begin of einde van een methode of een ander blok staat, dan is dat prima en kan ik afzien van verder onderzoek. Zie:
Enkele blokverklaring in een methode

Op dit punt gebruik ik al de PSI-nodes, omdat ik witruimte-informatie wil controleren die niet in UAST wordt geleverd. Daarvoor moet ik de sourcePsi eigenschap van de block level UAST node ophalen.

Het hebben van een commentaar voor een “block statement”

Een randgeval is als er een commentaar net boven mijn block statement staat. Hier wil ik de witruimte boven het commentaar controleren, dus de eerste relevante witruimte is boven het commentaarstatement.

  • Tot slot tel ik het aantal regeleindes in de relevante witruimte; als het niet groter is dan 1, meld ik een probleem.

Het resultaat

Als resultaat verwacht ik de volgende waarschuwingen in de IDE, zodat het voor elke ontwikkelaar duidelijk is dat er een extra regel moet worden toegevoegd. Ik kan deze waarschuwingen ook in het Lint rapport krijgen als ik lint gradle task uitvoer. Verder kan ik zelfs de ernst van het probleem verhogen naar error, als ik de merge request job wil blokkeren.

IDE waarschuwing voor en na blokstatement

Conclusie

Na de integratie van de aangepaste regel, hoeven we ons niet meer bezig te houden met dit frustrerende probleem; In plaats daarvan kunnen we onze denkkracht besteden aan het vinden van complexere problemen wanneer we elkaars code beoordelen. Nog beter, we kunnen 100% garanderen dat deze fout niet per ongeluk in onze codebase terechtkomt, omdat we onze MR job zo kunnen configureren dat hij faalt als iemand mist om dit probleem op te lossen.

Hoewel Android Lint niet primair voor opmaakregels is, was het in mijn geval toch heel praktisch, omdat het zowel voor Java als Kotlin kan worden gebruikt: geen dubbele regels schrijven, onderhouden en integreren nodig.

Aan de andere kant moeten we opmerken dat deze regel heel eenvoudig is, in die zin dat ik alleen witregels en accolades op PSI-niveau hoef te controleren, die in de twee talen hetzelfde zijn. Daarom hoef ik geen taalspecifieke code te schrijven. Echter, als ik moet nog wat taalspecifieke code te schrijven (bijvoorbeeld het omgaan Elvis operator in Kotlin of Ternary operator in Java), zou ik nog steeds overwegen om Android Lint te verkiezen boven het schrijven van een-one ktlint en een Checkstyle regels, omdat ik nog steeds veel minder werk zou hebben waarschijnlijk.

Volgende deel…

Als het eerste deel van het artikel je beviel, bekijk dan ook het tweede deel, waar ik de integratie van mijn regels in Android (en niet-Android) projecten in detail zal beschrijven, hoe we de reeds bestaande problemen in onze codebase kunnen oplossen, en hoe we onze regels kunnen unit-testen en debuggen.

Geef een reactie

Het e-mailadres wordt niet gepubliceerd. Vereiste velden zijn gemarkeerd met *