Skip to content

Commit

Permalink
[style](fe)the last step of fe CheckStyle (apache#10134)
Browse files Browse the repository at this point in the history
1. fix all checkstyle warning
2. change all checkstyle rules to error
3. remove some java doc rules
    a. RequireEmptyLineBeforeBlockTagGroup
    b. JavadocStyle
    c. JavadocParagraph
4. suppress some rules for old codes
    a. all java doc rules only affect on Nereids
    b. DeclarationOrder only affect on Nereids
    c. OverloadMethodsDeclarationOrder only affect on Nereids
    d. VariableDeclarationUsageDistance only affect on Nereids
    e. suppress OneTopLevelClass on org/apache/doris/load/loadv2/dpp/ColumnParser.java
    f. suppress OneTopLevelClass on org/apache/doris/load/loadv2/dpp/SparkRDDAggregator.java
    g. suppress LineLength on org/apache/doris/catalog/FunctionSet.java
    h. suppress LineLength on org/apache/doris/common/ErrorCode.java
  • Loading branch information
morrySnow authored Jun 17, 2022
1 parent fea815f commit b7b78ae
Show file tree
Hide file tree
Showing 514 changed files with 3,883 additions and 2,763 deletions.
7 changes: 7 additions & 0 deletions docs/en/developer/developer-guide/java-format-code.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ standard java package
* Do not use `import *`
* Do not use `import static`

## Check when compile

Now, when compiling with `caven`, `CheckStyle` checks are done by default. This will slightly slow down compilation. If you want to skip checkstyle, please use the following command to compile
```
mvn clean install -DskipTests -Dcheckstyle.skip
```

## Checkstyle Plugin

Now we have `formatter-check` in `CI` to check the code format.
Expand Down
7 changes: 7 additions & 0 deletions docs/zh-CN/developer/developer-guide/java-format-code.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ standard java package
* 禁止使用 `import *`
* 禁止使用 `import static`

## 编译时检查

现在,在使用`maven`进行编译时,会默认进行`CheckStyle`检查。此检查会略微降低编译速度。如果想跳过此检查,请使用如下命令进行编译
```
mvn clean install -DskipTests -Dcheckstyle.skip
```

## Checkstyle 插件

现在的 `CI` 之中会有 `formatter-check` 进行代码格式化检测。
Expand Down
119 changes: 21 additions & 98 deletions fe/check/checkstyle/checkstyle.xml

Large diffs are not rendered by default.

29 changes: 29 additions & 0 deletions fe/check/checkstyle/suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,33 @@ under the License.
<suppress files="[\\/]jmockit[\\/]" checks=".*" />
<suppress files="[\\/]test[\\/]" checks="MissingJavadocMethod" />
<suppress files="[\\/]test[\\/]" checks="MissingJavadocType" />
<suppress files="[\\/]test[\\/]" checks="LineLength" />

<!-- Suppress some rules except nereids -->
<!-- Java doc -->
<suppress files="org[\\/]apache[\\/]doris[\\/](?!nereids)[^\\/]+[\\/]|PaloFe\.java" checks="AtclauseOrder" />
<suppress files="org[\\/]apache[\\/]doris[\\/](?!nereids)[^\\/]+[\\/]|PaloFe\.java" checks="JavadocMethod" />
<suppress files="org[\\/]apache[\\/]doris[\\/](?!nereids)[^\\/]+[\\/]|PaloFe\.java" checks="JavadocParagraph" />
<suppress files="org[\\/]apache[\\/]doris[\\/](?!nereids)[^\\/]+[\\/]|PaloFe\.java" checks="JavadocStyle" />
<suppress files="org[\\/]apache[\\/]doris[\\/](?!nereids)[^\\/]+[\\/]|PaloFe\.java" checks="JavadocTagContinuationIndentation" />
<suppress files="org[\\/]apache[\\/]doris[\\/](?!nereids)[^\\/]+[\\/]|PaloFe\.java" checks="InvalidJavadocPosition" />
<suppress files="org[\\/]apache[\\/]doris[\\/](?!nereids)[^\\/]+[\\/]|PaloFe\.java" checks="MissingJavadocMethod" />
<suppress files="org[\\/]apache[\\/]doris[\\/](?!nereids)[^\\/]+[\\/]|PaloFe\.java" checks="MissingJavadocType" />
<suppress files="org[\\/]apache[\\/]doris[\\/](?!nereids)[^\\/]+[\\/]|PaloFe\.java" checks="NonEmptyAtclauseDescription" />
<suppress files="org[\\/]apache[\\/]doris[\\/](?!nereids)[^\\/]+[\\/]|PaloFe\.java" checks="RequireEmptyLineBeforeBlockTagGroup" />
<suppress files="org[\\/]apache[\\/]doris[\\/](?!nereids)[^\\/]+[\\/]|PaloFe\.java" checks="SummaryJavadoc" />
<suppress files="org[\\/]apache[\\/]doris[\\/](?!nereids)[^\\/]+[\\/]|PaloFe\.java" checks="SingleLineJavadoc" />

<!-- other -->
<suppress files="org[\\/]apache[\\/]doris[\\/](?!nereids)[^\\/]+[\\/]|PaloFe\.java" checks="DeclarationOrder" />
<suppress files="org[\\/]apache[\\/]doris[\\/](?!nereids)[^\\/]+[\\/]|PaloFe\.java" checks="OverloadMethodsDeclarationOrder" />
<suppress files="org[\\/]apache[\\/]doris[\\/](?!nereids)[^\\/]+[\\/]|PaloFe\.java" checks="VariableDeclarationUsageDistance" />

<!-- exclude rules for special files -->
<suppress files="org[\\/]apache[\\/]doris[\\/]load[\\/]loadv2[\\/]dpp[\\/]ColumnParser\.java" checks="OneTopLevelClass" />
<suppress files="org[\\/]apache[\\/]doris[\\/]load[\\/]loadv2[\\/]dpp[\\/]SparkRDDAggregator\.java" checks="OneTopLevelClass" />
<suppress files="org[\\/]apache[\\/]doris[\\/]catalog[\\/]FunctionSet\.java" checks="LineLength" />
<suppress files="org[\\/]apache[\\/]doris[\\/]common[\\/]ErrorCode\.java" checks="LineLength" />
<suppress files="org[\\/]apache[\\/]doris[\\/]udf[\\/]UdafExecutor\.java" checks="NoFinalizer" />
<suppress files="org[\\/]apache[\\/]doris[\\/]udf[\\/]UdfExecutor\.java" checks="NoFinalizer" />
</suppressions>
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public Buffer(int size) {
public void write(DataInput in, int len) throws IOException {
int newcount = count + len;
if (newcount > buf.length) {
byte newbuf[] = new byte[Math.max(buf.length << 1, newcount)];
byte[] newbuf = new byte[Math.max(buf.length << 1, newcount)];
System.arraycopy(buf, 0, newbuf, 0, count);
buf = newbuf;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
public class IOUtils {
public static long copyBytes(InputStream in, OutputStream out,
int buffSize, long len) throws IOException {
byte buf[] = new byte[buffSize];
byte[] buf = new byte[buffSize];
int totalRead = 0;
int toRead = 0;
int bytesRead = 0;
Expand Down Expand Up @@ -76,7 +76,7 @@ public static long copyBytes(InputStream in, OutputStream out,
int buffSize, int speed, boolean close) throws IOException {

PrintStream ps = out instanceof PrintStream ? (PrintStream) out : null;
byte buf[] = new byte[buffSize];
byte[] buf = new byte[buffSize];
long bytesReadTotal = 0;
long startTime = 0;
long sleepTime = 0;
Expand Down Expand Up @@ -133,7 +133,7 @@ public static long copyBytes(InputStream in, OutputStream out,
int buffSize, boolean close) throws IOException {

PrintStream ps = out instanceof PrintStream ? (PrintStream) out : null;
byte buf[] = new byte[buffSize];
byte[] buf = new byte[buffSize];
long totalBytes = 0;
try {
int bytesRead = in.read(buf);
Expand Down Expand Up @@ -169,7 +169,7 @@ public static long copyBytes(InputStream in, OutputStream out,
* if it could not read requested number of bytes for any reason
* (including EOF)
*/
public static void readFully(InputStream in, byte buf[], int off, int len)
public static void readFully(InputStream in, byte[] buf, int off, int len)
throws IOException {
int toRead = len;
int tmpOff = off;
Expand Down Expand Up @@ -263,6 +263,7 @@ public static void writeOptionString(DataOutput output, String value) throws IOE
Text.writeString(output, value);
}
}

public static String readOptionStringOrNull(DataInput input) throws IOException {
if (input.readBoolean()) {
return Text.readString(input);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public void reset() {
public void write(InputStream in, int len) throws IOException {
int newcount = count + len;
if (newcount > buf.length) {
byte newbuf[] = new byte[Math.max(buf.length << 1, newcount)];
byte[] newbuf = new byte[Math.max(buf.length << 1, newcount)];
System.arraycopy(buf, 0, newbuf, 0, count);
buf = newbuf;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ public static int bytesToCodePoint(ByteBuffer bytes) {
return ch;
}

static final int offsetsFromUTF8[] = { 0x00000000, 0x00003080, 0x000E2080,
static final int[] offsetsFromUTF8 = { 0x00000000, 0x00003080, 0x000E2080,
0x03C82080, 0xFA082080, 0x82082080 };

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ private String formatError(String rawVal) {
}
}

private static abstract class ComparableProperty<T extends Comparable> extends PropertySchema<T> {
private abstract static class ComparableProperty<T extends Comparable> extends PropertySchema<T> {
protected ComparableProperty(String name) {
super(name);
}
Expand Down
6 changes: 4 additions & 2 deletions fe/fe-core/src/main/java/org/apache/doris/PaloFe.java
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ public static void start(String dorisHomeDir, String pidDir, String[] args) {
// 1. HttpServer for HTTP Server
// 2. FeServer for Thrift Server
// 3. QeService for MySQL Server
QeService qeService = new QeService(Config.query_port, Config.mysql_service_nio_enabled, ExecuteEnv.getInstance().getScheduler());
QeService qeService = new QeService(Config.query_port, Config.mysql_service_nio_enabled,
ExecuteEnv.getInstance().getScheduler());
FeServer feServer = new FeServer(Config.rpc_port);

feServer.start();
Expand Down Expand Up @@ -324,7 +325,8 @@ private static void checkCommandLineOptions(CommandLineOptions cmdLineOpts) {
} else if (cmdLineOpts.runImageTool()) {
File imageFile = new File(cmdLineOpts.getImagePath());
if (!imageFile.exists()) {
System.out.println("image does not exist: " + imageFile.getAbsolutePath() + " . Please put an absolute path instead");
System.out.println("image does not exist: " + imageFile.getAbsolutePath()
+ " . Please put an absolute path instead");
System.exit(-1);
} else {
System.out.println("Start to load image: ");
Expand Down
33 changes: 22 additions & 11 deletions fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ private boolean processAlterOlapTable(AlterTableStmt stmt, OlapTable olapTable,
}
Catalog.getCurrentCatalog().dropPartition(db, olapTable, ((DropPartitionClause) alterClause));
} else if (alterClause instanceof ReplacePartitionClause) {
Catalog.getCurrentCatalog().replaceTempPartition(db, olapTable, (ReplacePartitionClause) alterClause);
Catalog.getCurrentCatalog().replaceTempPartition(
db, olapTable, (ReplacePartitionClause) alterClause);
} else if (alterClause instanceof ModifyPartitionClause) {
ModifyPartitionClause clause = ((ModifyPartitionClause) alterClause);
// expand the partition names if it is 'Modify Partition(*)'
Expand Down Expand Up @@ -206,7 +207,8 @@ private boolean processAlterOlapTable(AlterTableStmt stmt, OlapTable olapTable,
} else if (currentAlterOps.contains(AlterOpType.MODIFY_DISTRIBUTION)) {
Preconditions.checkState(alterClauses.size() == 1);
AlterClause alterClause = alterClauses.get(0);
Catalog.getCurrentCatalog().modifyDefaultDistributionBucketNum(db, olapTable, (ModifyDistributionClause) alterClause);
Catalog.getCurrentCatalog().modifyDefaultDistributionBucketNum(
db, olapTable, (ModifyDistributionClause) alterClause);
} else if (currentAlterOps.contains(AlterOpType.MODIFY_COLUMN_COMMENT)) {
processModifyColumnComment(db, olapTable, alterClauses);
} else if (currentAlterOps.contains(AlterOpType.MODIFY_TABLE_COMMENT)) {
Expand All @@ -227,7 +229,8 @@ private void processModifyTableComment(Database db, OlapTable tbl, AlterClause a
ModifyTableCommentClause clause = (ModifyTableCommentClause) alterClause;
tbl.setComment(clause.getComment());
// log
ModifyCommentOperationLog op = ModifyCommentOperationLog.forTable(db.getId(), tbl.getId(), clause.getComment());
ModifyCommentOperationLog op = ModifyCommentOperationLog
.forTable(db.getId(), tbl.getId(), clause.getComment());
Catalog.getCurrentCatalog().getEditLog().logModifyComment(op);
} finally {
tbl.writeUnlock();
Expand Down Expand Up @@ -338,7 +341,8 @@ public void replayProcessModifyEngine(ModifyTableEngineOperationLog log) {
}
}

private void processModifyEngineInternal(Database db, Table externalTable, Map<String, String> prop, boolean isReplay) {
private void processModifyEngineInternal(Database db, Table externalTable,
Map<String, String> prop, boolean isReplay) {
MysqlTable mysqlTable = (MysqlTable) externalTable;
Map<String, String> newProp = Maps.newHashMap(prop);
newProp.put(OdbcTable.ODBC_HOST, mysqlTable.getHost());
Expand Down Expand Up @@ -393,7 +397,8 @@ public void processAlterTable(AlterTableStmt stmt) throws UserException {
processAlterExternalTable(stmt, table, db);
return;
default:
throw new DdlException("Do not support alter " + table.getType().toString() + " table[" + tableName + "]");
throw new DdlException("Do not support alter "
+ table.getType().toString() + " table[" + tableName + "]");
}

// the following ops should done outside table lock. because it contain synchronized create operation
Expand All @@ -402,7 +407,8 @@ public void processAlterTable(AlterTableStmt stmt) throws UserException {
AlterClause alterClause = alterClauses.get(0);
if (alterClause instanceof AddPartitionClause) {
if (!((AddPartitionClause) alterClause).isTempPartition()) {
DynamicPartitionUtil.checkAlterAllowed((OlapTable) db.getTableOrMetaException(tableName, TableType.OLAP));
DynamicPartitionUtil.checkAlterAllowed(
(OlapTable) db.getTableOrMetaException(tableName, TableType.OLAP));
}
Catalog.getCurrentCatalog().addPartition(db, tableName, (AddPartitionClause) alterClause);
} else if (alterClause instanceof ModifyPartitionClause) {
Expand Down Expand Up @@ -432,7 +438,8 @@ public void processAlterTable(AlterTableStmt stmt) throws UserException {
}

// entry of processing replace table
private void processReplaceTable(Database db, OlapTable origTable, List<AlterClause> alterClauses) throws UserException {
private void processReplaceTable(Database db, OlapTable origTable, List<AlterClause> alterClauses)
throws UserException {
ReplaceTableClause clause = (ReplaceTableClause) alterClauses.get(0);
String newTblName = clause.getTblName();
boolean swapTable = clause.isSwapTable();
Expand All @@ -452,7 +459,8 @@ private void processReplaceTable(Database db, OlapTable origTable, List<AlterCla
}
replaceTableInternal(db, origTable, olapNewTbl, swapTable, false);
// write edit log
ReplaceTableOperationLog log = new ReplaceTableOperationLog(db.getId(), origTable.getId(), olapNewTbl.getId(), swapTable);
ReplaceTableOperationLog log = new ReplaceTableOperationLog(db.getId(),
origTable.getId(), olapNewTbl.getId(), swapTable);
Catalog.getCurrentCatalog().getEditLog().logReplaceTable(log);
LOG.info("finish replacing table {} with table {}, is swap: {}", oldTblName, newTblName, swapTable);
} finally {
Expand Down Expand Up @@ -533,7 +541,8 @@ public void processAlterView(AlterViewStmt stmt, ConnectContext ctx) throws User
modifyViewDef(db, view, stmt.getInlineViewDef(), ctx.getSessionVariable().getSqlMode(), stmt.getColumns());
}

private void modifyViewDef(Database db, View view, String inlineViewDef, long sqlMode, List<Column> newFullSchema) throws DdlException {
private void modifyViewDef(Database db, View view, String inlineViewDef, long sqlMode,
List<Column> newFullSchema) throws DdlException {
db.writeLockOrDdlException();
try {
view.writeLockOrDdlException();
Expand All @@ -549,7 +558,8 @@ private void modifyViewDef(Database db, View view, String inlineViewDef, long sq
db.dropTable(viewName);
db.createTable(view);

AlterViewInfo alterViewInfo = new AlterViewInfo(db.getId(), view.getId(), inlineViewDef, newFullSchema, sqlMode);
AlterViewInfo alterViewInfo = new AlterViewInfo(db.getId(), view.getId(),
inlineViewDef, newFullSchema, sqlMode);
Catalog.getCurrentCatalog().getEditLog().logModifyViewDef(alterViewInfo);
LOG.info("modify view[{}] definition to {}", viewName, inlineViewDef);
} finally {
Expand Down Expand Up @@ -680,7 +690,8 @@ public void modifyPartitionsProperty(Database db,
DateLiteral dateLiteral = new DateLiteral(dataProperty.getCooldownTimeMs(),
TimeUtils.getTimeZone(), Type.DATETIME);
newProperties.put(PropertyAnalyzer.PROPERTIES_STORAGE_COOLDOWN_TIME, dateLiteral.getStringValue());
newProperties.put(PropertyAnalyzer.PROPERTIES_REMOTE_STORAGE_RESOURCE, dataProperty.getRemoteStorageResourceName());
newProperties.put(PropertyAnalyzer.PROPERTIES_REMOTE_STORAGE_RESOURCE,
dataProperty.getRemoteStorageResourceName());
DateLiteral dateLiteral1 = new DateLiteral(dataProperty.getRemoteCooldownTimeMs(),
TimeUtils.getTimeZone(), Type.DATETIME);
newProperties.put(PropertyAnalyzer.PROPERTIES_REMOTE_STORAGE_COOLDOWN_TIME, dateLiteral1.getStringValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ private void clearExpireFinishedOrCancelledAlterJobsV2() {
AlterJobV2 alterJobV2 = iterator.next().getValue();
if (alterJobV2.isExpire()) {
iterator.remove();
RemoveAlterJobV2OperationLog log = new RemoveAlterJobV2OperationLog(alterJobV2.getJobId(), alterJobV2.getType());
RemoveAlterJobV2OperationLog log = new RemoveAlterJobV2OperationLog(
alterJobV2.getJobId(), alterJobV2.getType());
Catalog.getCurrentCatalog().getEditLog().logRemoveExpiredAlterJobV2(log);
LOG.info("remove expired {} job {}. finish at {}", alterJobV2.getType(),
alterJobV2.getJobId(), TimeUtils.longToTimeString(alterJobV2.getFinishedTimeMs()));
Expand Down Expand Up @@ -169,7 +170,7 @@ public abstract void process(List<AlterClause> alterClauses, String clusterName,
* entry function. handle alter ops for external table
*/
public void processExternalTable(List<AlterClause> alterClauses, Database db, Table externalTable)
throws UserException {};
throws UserException {}

/*
* cancel alter ops
Expand All @@ -183,11 +184,13 @@ public void processExternalTable(List<AlterClause> alterClauses, Database db, Ta
* We assume that the specified version is X.
* Case 1:
* After alter table process starts, there is no new load job being submitted. So the new replica
* should be with version (0-1). So we just modify the replica's version to partition's visible version, which is X.
* should be with version (0-1). So we just modify the replica's version to
* partition's visible version, which is X.
* Case 2:
* After alter table process starts, there are some load job being processed.
* Case 2.1:
* None of them succeed on this replica. so the version is still 1. We should modify the replica's version to X.
* None of them succeed on this replica. so the version is still 1.
* We should modify the replica's version to X.
* Case 2.2
* There are new load jobs after alter task, and at least one of them is succeed on this replica.
* So the replica's version should be larger than X. So we don't need to modify the replica version
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,12 @@
public abstract class AlterJobV2 implements Writable {
private static final Logger LOG = LogManager.getLogger(AlterJobV2.class);


public enum JobState {
PENDING, // Job is created
// CHECKSTYLE OFF
WAITING_TXN, // New replicas are created and Shadow catalog object is visible for incoming txns, waiting for previous txns to be finished
// CHECKSTYLE ON
RUNNING, // alter tasks are sent to BE, and waiting for them finished.
FINISHED, // job is done
CANCELLED; // job is cancelled(failed or be cancelled by user)
Expand Down Expand Up @@ -175,7 +178,7 @@ public synchronized void run() {
}
}

public synchronized final boolean cancel(String errMsg) {
public final synchronized boolean cancel(String errMsg) {
return cancelImpl(errMsg);
}

Expand Down
Loading

0 comments on commit b7b78ae

Please sign in to comment.