[Bug][alert] WeChatSender.mkString() contains dead code — StringUtils.isEmpty("|") always returns false

[asadjan4611]

Search before asking

• I had searched in the issues and found no similar issues.

What happened

In WeChatSender.java, the mkString() method contains a dead-code condition:

private static String mkString(Iterable<String> list) {
    if (null == list || StringUtils.isEmpty("|")) {
        return null;
    }
    ...
}

StringUtils.isEmpty("|") always evaluates to false because "|" is a non-empty string literal. This means the second condition in the || check is dead code. The "|" literal was likely meant to be a variable or parameter representing the separator — possibly from a refactor or copy-paste error.

File: dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-wechat/src/main/java/org/apache/dolphinscheduler/plugin/alert/wechat/WeChatSender.java (line ~163)

What you expected to happen

The condition should check a meaningful value. Either:

• The separator should be a method parameter: mkString(Iterable<String> list, String separator)
• Or the dead-code condition should be removed entirely, since the separator "|" is hardcoded and always valid.

How to reproduce

1. Read the source code of WeChatSender.java, method mkString() at line 163.
2. Observe that StringUtils.isEmpty("|") is always false — this is verifiable by running any Java expression evaluator.
3. The dead branch return null due to the second condition can never be reached.

Anything else

This is a static code analysis bug. It does not cause a runtime crash, but it indicates a logic defect: the separator was likely intended to be configurable or passed as a parameter. The method only works correctly by accident because the hardcoded "|" is non-empty.

Version

dev

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[SbloodyS]

This function mkString is no usage anymore. So it won't affect any function. Why do you think this is a bug?

[asadjan4611]

Thank you for the review. You're right — I verified that mkString() has no callers in the codebase, so it's indeed unused dead code rather than a functional bug.

Given that, I'd like to update this issue from [Bug] to [Improvement]: remove the unused mkString() method entirely as dead code cleanup. This would:

• Remove dead code that could confuse future contributors
• Eliminate the misleading StringUtils.isEmpty("|") that static analysis tools flag
• Allow the unused import org.apache.commons.lang3.StringUtils to be removed (if no other usage exists in the file)

Would you prefer I update the PR to simply delete the mkString() method instead?