Skip to content

GitLab

A
appinventor-sources
Unverified Commit 11512a65 authored by Evan W. Patton's avatar Evan W. Patton Committed by GitHub
Browse files
Options

Implement checkstyle rules for App Inventor (#2149) 

* Implement checkstyle rules for App Inventor

This commit implements checkstyle for App Inventor's Java code. The
App Inventor style rules are based on the Google style rules, with an
exception added for methods in the components/runtime package that are
annotated with `@Simple*` or `@Design*` annotations. The App Inventor
specific rules are placed at the end of the appinventor_checks.xml
file.

In addition, there is a script called checkstyle_git.py that is
written in Python 3 and will check the git diff against the rules by
running checkstyle on the changed files and crossreferencing the line
numbers with the checkstyle output. This can be used as a git
pre-commit hook to check that changes conform to the style guide. The
script returns -1 on error, which will abort the commit until the
changes confirm. This behavior can be bypassed by exporting the
BYPASS_CHECKSTYLE environment variable.

These two changes are made available as ant targets:

- `ant checkstyle`: Run the checkstyle_git.py script to check the
  git diff for problems.
- `ant checkstyle-all`: Run checkstyle on the whole repo (slow).

Change-Id: I26c6049bd49d1ce3f9b8dbd48f8f115a89fb5b52

* Revamp checkstyle-git script logic

Change-Id: Ie417057b2d009ba70279e70df102851b2f4552cd

* Add custom checks for EventDispatcher in @SimpleEvent

Change-Id: I72a704e9f8d489aa08cefd50ca180b1130b3e5fa
parent e1122b90
Hide whitespace changes
Inline Side-by-side
Showing with 441 additions and 0 deletions
appinventor/build.xml
View file @ 11512a65
......@@ -129,6 +129,35 @@
</javadoc>
</target>
<target name="checkstyle-all">
<java classname="com.puppycrawl.tools.checkstyle.Main" fork="true">
<classpath>
<pathelement path="lib/checkstyle/checkstyle.jar"/>
<pathelement path="lib/checkstyle/appinventor-checks.jar"/>
</classpath>
<arg value="-c"/>
<arg value="lib/checkstyle/appinventor-checks.xml"/>
<arg value="appengine/src"/>
<arg value="appengine/tests"/>
<arg value="blocklyeditor/src"/>
<arg value="blocklyeditor/tests"/>
<arg value="buildserver/src"/>
<arg value="buildserver/tests"/>
<arg value="common/src"/>
<arg value="common/tests"/>
<arg value="components/src"/>
<arg value="components/tests"/>
</java>
</target>
<target name="checkstyle">
<property name="commit" value="HEAD"/>
<exec executable="misc/checkstyle/checkstyle-git.py"
failonerror="true">
<arg value="${commit}"/>
</exec>
</target>
<target name="clean">
<ant inheritAll="false" useNativeBasedir="true" dir="appengine" target="clean"/>
<ant inheritAll="false" useNativeBasedir="true" dir="blocklyeditor" target="clean"/>
......
appinventor/lib/checkstyle/appinventor-checks.xml 0 → 100644
View file @ 11512a65
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
"-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
"https://checkstyle.org/dtds/configuration_1_3.dtd">
<!--
Checkstyle configuration that checks the Google coding conventions from Google Java Style
that can be found at https://google.github.io/styleguide/javaguide.html.
Checkstyle is very configurable. Be sure to read the documentation at
http://checkstyle.sf.net (or in your downloaded distribution).
To completely disable a check, just comment it out or delete it from the file.
Authors: Max Vetrenko, Ruslan Diachenko, Roman Ivanov.
-->
<module name = "Checker">
<property name="charset" value="UTF-8"/>
<property name="severity" value="warning"/>
<property name="fileExtensions" value="java, properties, xml"/>
<!-- Checks for whitespace -->
<!-- See http://checkstyle.sf.net/config_whitespace.html -->
<module name="FileTabCharacter">
<property name="eachLine" value="true"/>
</module>
<module name="TreeWalker">
<module name="OuterTypeFilename"/>
<module name="IllegalTokenText">
<property name="tokens" value="STRING_LITERAL, CHAR_LITERAL"/>
<property name="format"
value="\\u00(09|0(a|A)|0(c|C)|0(d|D)|22|27|5(C|c))|\\(0(10|11|12|14|15|42|47)|134)"/>
<property name="message"
value="Consider using special escape sequence instead of octal value or Unicode escaped value."/>
</module>
<module name="AvoidEscapedUnicodeCharacters">
<property name="allowEscapesForControlCharacters" value="true"/>
<property name="allowByTailComment" value="true"/>
<property name="allowNonPrintableEscapes" value="true"/>
</module>
<module name="LineLength">
<property name="max" value="100"/>
<property name="ignorePattern" value="^package.*|^import.*|a href|href|http://|https://|ftp://"/>
</module>
<module name="AvoidStarImport"/>
<module name="OneTopLevelClass"/>
<module name="NoLineWrap"/>
<module name="EmptyBlock">
<property name="option" value="TEXT"/>
<property name="tokens"
value="LITERAL_TRY, LITERAL_FINALLY, LITERAL_IF, LITERAL_ELSE, LITERAL_SWITCH"/>
</module>
<module name="NeedBraces"/>
<module name="LeftCurly"/>
<module name="RightCurly">
<property name="id" value="RightCurlySame"/>
<property name="tokens"
value="LITERAL_TRY, LITERAL_CATCH, LITERAL_FINALLY, LITERAL_IF, LITERAL_ELSE,
LITERAL_DO"/>
</module>
<module name="RightCurly">
<property name="id" value="RightCurlyAlone"/>
<property name="option" value="alone"/>
<property name="tokens"
value="CLASS_DEF, METHOD_DEF, CTOR_DEF, LITERAL_FOR, LITERAL_WHILE, STATIC_INIT,
INSTANCE_INIT"/>
</module>
<module name="WhitespaceAround">
<property name="allowEmptyConstructors" value="true"/>
<property name="allowEmptyLambdas" value="true"/>
<property name="allowEmptyMethods" value="true"/>
<property name="allowEmptyTypes" value="true"/>
<property name="allowEmptyLoops" value="true"/>
<message key="ws.notFollowed"
value="WhitespaceAround: ''{0}'' is not followed by whitespace. Empty blocks may only be represented as '{}' when not part of a multi-block statement (4.1.3)"/>
<message key="ws.notPreceded"
value="WhitespaceAround: ''{0}'' is not preceded with whitespace."/>
</module>
<module name="OneStatementPerLine"/>
<module name="MultipleVariableDeclarations"/>
<module name="ArrayTypeStyle"/>
<module name="MissingSwitchDefault"/>
<module name="FallThrough"/>
<module name="UpperEll"/>
<module name="ModifierOrder"/>
<module name="EmptyLineSeparator">
<property name="allowNoEmptyLineBetweenFields" value="true"/>
</module>
<module name="SeparatorWrap">
<property name="id" value="SeparatorWrapDot"/>
<property name="tokens" value="DOT"/>
<property name="option" value="nl"/>
</module>
<module name="SeparatorWrap">
<property name="id" value="SeparatorWrapComma"/>
<property name="tokens" value="COMMA"/>
<property name="option" value="EOL"/>
</module>
<module name="SeparatorWrap">
<!-- ELLIPSIS is EOL until https://github.com/google/styleguide/issues/258 -->
<property name="id" value="SeparatorWrapEllipsis"/>
<property name="tokens" value="ELLIPSIS"/>
<property name="option" value="EOL"/>
</module>
<module name="SeparatorWrap">
<!-- ARRAY_DECLARATOR is EOL until https://github.com/google/styleguide/issues/259 -->
<property name="id" value="SeparatorWrapArrayDeclarator"/>
<property name="tokens" value="ARRAY_DECLARATOR"/>
<property name="option" value="EOL"/>
</module>
<module name="SeparatorWrap">
<property name="id" value="SeparatorWrapMethodRef"/>
<property name="tokens" value="METHOD_REF"/>
<property name="option" value="nl"/>
</module>
<module name="PackageName">
<property name="format" value="^[a-z]+(\.[a-z][a-z0-9]*)*$"/>
<message key="name.invalidPattern"
value="Package name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="TypeName">
<message key="name.invalidPattern"
value="Type name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="MemberName">
<property name="format" value="^[a-z][a-z0-9][a-zA-Z0-9]*$"/>
<message key="name.invalidPattern"
value="Member name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="ParameterName">
<property name="format" value="^[a-z]([a-z0-9][a-zA-Z0-9]*)?$"/>
<message key="name.invalidPattern"
value="Parameter name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="LambdaParameterName">
<property name="format" value="^[a-z]([a-z0-9][a-zA-Z0-9]*)?$"/>
<message key="name.invalidPattern"
value="Lambda parameter name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="CatchParameterName">
<property name="format" value="^[a-z]([a-z0-9][a-zA-Z0-9]*)?$"/>
<message key="name.invalidPattern"
value="Catch parameter name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="LocalVariableName">
<property name="tokens" value="VARIABLE_DEF"/>
<property name="format" value="^[a-z]([a-z0-9][a-zA-Z0-9]*)?$"/>
<message key="name.invalidPattern"
value="Local variable name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="ClassTypeParameterName">
<property name="format" value="(^[A-Z][0-9]?)$|([A-Z][a-zA-Z0-9]*[T]$)"/>
<message key="name.invalidPattern"
value="Class type name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="MethodTypeParameterName">
<property name="format" value="(^[A-Z][0-9]?)$|([A-Z][a-zA-Z0-9]*[T]$)"/>
<message key="name.invalidPattern"
value="Method type name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="InterfaceTypeParameterName">
<property name="format" value="(^[A-Z][0-9]?)$|([A-Z][a-zA-Z0-9]*[T]$)"/>
<message key="name.invalidPattern"
value="Interface type name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="NoFinalizer"/>
<module name="GenericWhitespace">
<message key="ws.followed"
value="GenericWhitespace ''{0}'' is followed by whitespace."/>
<message key="ws.preceded"
value="GenericWhitespace ''{0}'' is preceded with whitespace."/>
<message key="ws.illegalFollow"
value="GenericWhitespace ''{0}'' should followed by whitespace."/>
<message key="ws.notPreceded"
value="GenericWhitespace ''{0}'' is not preceded with whitespace."/>
</module>
<module name="Indentation">
<property name="basicOffset" value="2"/>
<property name="braceAdjustment" value="0"/>
<property name="caseIndent" value="2"/>
<property name="throwsIndent" value="4"/>
<property name="lineWrappingIndentation" value="4"/>
<property name="arrayInitIndent" value="2"/>
</module>
<module name="AbbreviationAsWordInName">
<property name="ignoreFinal" value="false"/>
<property name="allowedAbbreviationLength" value="1"/>
</module>
<module name="OverloadMethodsDeclarationOrder"/>
<module name="VariableDeclarationUsageDistance"/>
<module name="CustomImportOrder">
<property name="sortImportsInGroupAlphabetically" value="true"/>
<property name="separateLineBetweenGroups" value="true"/>
<property name="customImportOrderRules" value="STATIC###THIRD_PARTY_PACKAGE"/>
</module>
<module name="MethodParamPad"/>
<module name="NoWhitespaceBefore">
<property name="tokens"
value="COMMA, SEMI, POST_INC, POST_DEC, DOT, ELLIPSIS, METHOD_REF"/>
<property name="allowLineBreaks" value="true"/>
</module>
<module name="ParenPad"/>
<module name="OperatorWrap">
<property name="option" value="NL"/>
<property name="tokens"
value="BAND, BOR, BSR, BXOR, DIV, EQUAL, GE, GT, LAND, LE, LITERAL_INSTANCEOF, LOR,
LT, MINUS, MOD, NOT_EQUAL, PLUS, QUESTION, SL, SR, STAR, METHOD_REF "/>
</module>
<module name="AnnotationLocation">
<property name="id" value="AnnotationLocationMostCases"/>
<property name="tokens"
value="CLASS_DEF, INTERFACE_DEF, ENUM_DEF, METHOD_DEF, CTOR_DEF"/>
</module>
<module name="AnnotationLocation">
<property name="id" value="AnnotationLocationVariables"/>
<property name="tokens" value="VARIABLE_DEF"/>
<property name="allowSamelineMultipleAnnotations" value="true"/>
</module>
<module name="NonEmptyAtclauseDescription"/>
<module name="JavadocTagContinuationIndentation"/>
<module name="SummaryJavadoc">
<property name="forbiddenSummaryFragments"
value="^@return the *|^This method returns |^A [{]@code [a-zA-Z0-9]+[}]( is a )"/>
</module>
<module name="JavadocParagraph"/>
<module name="AtclauseOrder">
<property name="tagOrder" value="@param, @return, @throws, @deprecated"/>
<property name="target"
value="CLASS_DEF, INTERFACE_DEF, ENUM_DEF, METHOD_DEF, CTOR_DEF, VARIABLE_DEF"/>
</module>
<module name="JavadocMethod">
<property name="scope" value="public"/>
<property name="allowMissingParamTags" value="true"/>
<property name="allowMissingThrowsTags" value="true"/>
<property name="allowMissingReturnTag" value="true"/>
<property name="minLineCount" value="2"/>
<property name="allowedAnnotations" value="Override, Test"/>
<property name="allowThrowsTagsForSubclasses" value="true"/>
</module>
<module name="SingleLineJavadoc">
<property name="ignoreInlineTags" value="false"/>
</module>
<module name="EmptyCatchBlock">
<property name="exceptionVariableName" value="expected"/>
</module>
<module name="CommentsIndentation"/>
<!-- App Inventor-specific checks -->
<module name="edu.mit.appinventor.checkstyle.checks.EventDispatchArguments"/>
<module name="edu.mit.appinventor.checkstyle.checks.EventDispatchMissing"/>
<module name="edu.mit.appinventor.checkstyle.checks.EventDispatchWrongName"/>
<module name="MethodName">
<property name="id" value="RegularMethodName"/>
<property name="format" value="^[a-z][a-z0-9][a-zA-Z0-9_]*$"/>
<message key="name.invalidPattern"
value="Method name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="MethodName">
<property name="id" value="AppInventorMethodName"/>
<property name="format" value="^[A-Z][a-zA-Z][a-zA-Z0-9]*$"/>
<message key="name.invalidPattern"
value="App Inventor method name ''{0}'' must match pattern ''{1}''."/>
</module>
<!-- Method naming suppression rules -->
<module name="SuppressionXpathSingleFilter">
<!-- Suppress regular method names in component runtime if the method is annotated with @Simple* or @Design* -->
<property name="files" value=".*[\\/]components[\\/]runtime[\\/].*"/>
<property name="id" value="RegularMethodName"/>
<property name="query" value="//METHOD_DEF[@text = 'Initialize' or ./MODIFIERS/ANNOTATION[starts-with(@text, 'Design') or starts-with(@text, 'Simple')]]/IDENT"/>
</module>
<module name="SuppressionXpathSingleFilter">
<!-- Suppress app inventor naming convention in component runtime if the method isn't annotated -->
<property name="files" value=".*[\\/]components[\\/]runtime[\\/].*"/>
<property name="id" value="AppInventorMethodName"/>
<property name="query" value="//METHOD_DEF[not(./MODIFIERS/ANNOTATION[starts-with(@text, 'Design') or starts-with(@text, 'Simple')])]/IDENT"/>
</module>
<module name="SuppressionXpathSingleFilter">
<!-- Only use regular method name format for other modules -->
<property name="files" value=".*[\\/](appengine|blocklyeditor|buildserver|common)[\\/](src|tests)[\\/].*"/>
<property name="id" value="AppInventorMethodName"/>
</module>
<module name="SuppressionXpathSingleFilter">
<!-- Only use regular method name format for components module except the runtime package -->
<property name="files" value=".*[\\/]google[\\/]appinventor[\\/]components[\\/][^r].*"/>
<property name="id" value="AppInventorMethodName"/>
</module>
<module name="SuppressWarningsHolder"/>
</module>
<module name="SuppressWarningsFilter"/>
</module>
appinventor/misc/checkstyle/checkstyle-git.py 0 → 100755
View file @ 11512a65
#!/usr/bin/env python3
import subprocess
from io import StringIO
from re import compile
import os
import sys
parent_commit = 'HEAD' if len(sys.argv) < 2 else sys.argv[1]
checkstyle_pattern = compile(r'^\[(?P<severity>[^\]]*)\] (?P<filename>[^:]*):(?P<line>[0-9]*):(?P<message>.*)'
r' \[(?P<rule>[^\]]*)\]$')
compile(r'^\[(?P<severity>[^\]]*)\]')
basedir = os.getcwd() if os.path.exists('build.xml') else os.path.join(os.getcwd(), 'appinventor')
def checkstyle(current_file):
return StringIO(subprocess.check_output(['java', '-cp',
'lib/checkstyle/checkstyle.jar:lib/checkstyle/appinventor-checks.jar',
'com.puppycrawl.tools.checkstyle.Main', '-c',
'lib/checkstyle/appinventor-checks.xml', current_file],
encoding='utf-8', cwd=basedir))
def check_chunks(checkstyle_output, chunks):
"""
:param checkstyle_output:
:param chunks:
:type chunks: list[(int, int)]
:return:
"""
success = True
i = 0
for line in checkstyle_output:
match = checkstyle_pattern.match(line.strip())
if match:
linenum = int(match.group('line'))
while i < len(chunks):
if chunks[i][0] <= linenum < chunks[i][1]:
print(line.strip())
success = False
break
elif chunks[i][0] <= linenum:
i += 1
if i >= len(chunks):
return success
else:
break
return success
def process_chunk_info(line):
"""
Processes a unified diff chunk header and returns a tuple indication the start and length of the deletion and
addition hunks.
:param line: Unified diff chunk marker, beginning with '@@'
:type line: str
:return: a 4-tuple of deletion start, deletion length, addition start, addition length
:rtype: tuple[int]
"""
parts = line.split(' ')
del_info = parts[1][1:]
add_info = parts[2][1:]
del_start, del_length = map(int, del_info.split(',')) if ',' in del_info else (int(del_info), 1)
add_start, add_length = map(int, add_info.split(',')) if ',' in add_info else (int(add_info), 1)
return del_start, del_length, add_start, add_length
def main(parent):
current_file, chunk_start, chunk_length = '', 0, 0
checkstyle_output = None
chunks = []
passed = True
for line in StringIO(subprocess.check_output(['git', 'diff', '-U0', parent], encoding='utf-8')):
if line.startswith('diff --git'):
pass
elif line.startswith('index '):
pass
elif line.startswith('---'):
if '/dev/null' in line:
new_file = True
else:
new_file = False
pass
elif line.startswith('+++'):
if line.startswith('+++ /dev/null'):
continue # Deleted file
if len(chunks) > 0 and current_file != '':
passed = check_chunks(checkstyle_output, chunks) and passed
# Handle new file
current_file = line[6:].strip()
current_file = os.path.join('..', current_file)
checkstyle_output = checkstyle(current_file)
chunks = []
elif line.startswith('@@'):
# Handle chunk
del_start, del_length, add_start, add_length = process_chunk_info(line)
if add_length > 0:
# Addition or replacement
chunks.append((add_start, add_start + add_length))
else:
# Code removal. Check next line to ensure it didn't introduce an error
chunks.append((del_start, del_start + 1))
if len(chunks) > 0:
passed = check_chunks(checkstyle_output, chunks) and passed
return passed
if __name__ == '__main__':
if 'BYPASS_CHECKSTYLE' in os.environ:
sys.exit(0)
if main(parent_commit):
sys.exit(0)
else:
sys.exit(1)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment