Decidi testar o analisador de código Java estático IntelliJ IDEA e, com ele, testei o projeto The Chemistry Development Kit . Aqui vou dar alguns erros que encontrei. Eu acho que alguns deles são típicos para programas Java como um todo, para que possam ser interessantes.
O Chemistry Development Kit é uma biblioteca Java de código aberto para resolver os problemas de quimioinformática e bioinformática. Quando eu estava envolvido em bioinformática, nós o usamos ativamente. O projeto está em desenvolvimento há mais de 20 anos, possui dezenas de autores e a qualidade do código é muito desigual. No entanto, existem testes de unidade no projeto, e a integração com o analisador de cobertura JaCoCo é prescrita em pom.xml . Além disso, plugins para três analisadores estáticos são configurados lá: FindBugs , PMD , Checkstyle . É ainda mais interessante verificar quais avisos permanecem.
O analisador de código Java estático incorporado no IntelliJ IDEA não é inferior às ferramentas especializadas de análise estática, mas, de certa forma, as ultrapassa. Além disso, praticamente todos os recursos de análise estática estão disponíveis no Community Edition , um IDE de código aberto gratuito. Em particular, a versão gratuita produz todos os avisos descritos neste artigo.
Por padrão, a análise estática é realizada constantemente no modo de edição de código; portanto, se você escrever um código no IntelliJ IDEA, corrigirá muitos erros literalmente em segundos após a execução, mesmo antes de executar os testes. Você pode verificar o projeto inteiro ou sua parte no modo de lote usando o botão Analisar | Inspecione o código ou execute uma inspeção separada usando o Analyze | Execute a inspeção por nome . Nesse caso, algumas inspeções ficam disponíveis que, devido à complexidade, não funcionam no modo de edição. No entanto, existem poucas inspeções.
Muitas inspeções do IntelliJ IDEA não relatam bugs, mas códigos imprecisos ou oferecem uma alternativa mais idiomática, bonita ou rápida. Isso é útil quando você está trabalhando constantemente no IDE. No entanto, no meu caso, é melhor começar com aquelas mensagens que alertam sobre erros reais. Basicamente, a categoria Java é interessante | Bugs prováveis , embora existam outras categorias que vale a pena explorar, como Problemas numéricos .
Vou falar apenas sobre alguns avisos interessantes.
1. Unário mais
Já havia 66 vantagens unárias no projeto. Para escrever +1 vez de apenas 1 às vezes eu quero ser bonita. No entanto, em alguns casos, o plus unário sai se, em vez de += escreverem =+ :
int totalCharge1 = 0; while (atoms1.hasNext()) { totalCharge1 = +((IAtom) atoms1.next()).getFormalCharge(); } Iterator<IAtom> atoms2 = products.atoms().iterator(); int totalCharge2 = 0; while (atoms2.hasNext()) { totalCharge2 = +((IAtom) atoms2.next()).getFormalCharge(); }
Um erro de digitação óbvio que ignora todas as iterações do loop, exceto a última. Pode parecer estranho que não esteja escrito “espaço igual a mais espaço”, mas “espaço igual a espaço mais”. No entanto, a estranheza desaparece se você se aprofundar na história . Inicialmente, "igual" e "mais" estavam realmente lá, mas em 2008 eles passaram por um formatador automático, e o código mudou. A propósito, aqui está a moral para os analisadores estáticos: é razoável emitir avisos com base em formatação estranha, mas se o código for formatado automaticamente, os avisos desaparecerão e os erros permanecerão.
2. Divisão inteira levando a fracionário
Um erro bastante irritante, mas os analisadores estáticos o acham bem. Aqui está um exemplo :
angle = 1 / 180 * Math.PI;
Infelizmente, o ângulo acabou não sendo um grau, mas zero. Erro semelhante :
Integer c1 = features1.get(key); Integer c2 = features2.get(key); c1 = c1 == null ? 0 : c1; c2 = c2 == null ? 0 : c2; sum += 1.0 - Math.abs(c1 - c2) / (c1 + c2);
Parece que os números c1 e c2 não c2 negativos, o que significa que o módulo de diferença nunca excederá a soma. Portanto, o resultado será 0 se os dois números forem diferentes de zero ou 1 se um deles for 0.
3. Chame Class.getClass ()
Às vezes, as pessoas chamam o método getClass() em um objeto do tipo Class . O resultado é novamente um objeto do tipo Class com o valor constante Class.class . Isso geralmente é um erro: getClass() não precisa ser chamado. Por exemplo, aqui :
public <T extends ICDKObject> T ofClass(Class<T> intf, Object... objects) { try { if (!intf.isInterface()) throw new IllegalArgumentException("expected interface, got " + intf.getClass()); ...
Se ocorrer uma exceção, o relatório será completamente inútil. A propósito, os erros no procedimento de tratamento de erros são frequentemente encontrados pela análise estática em projetos antigos: como regra, os procedimentos de tratamento de erros são testados de maneira pior.
4. Chame toString () em uma matriz
Este é um clássico do gênero: toString () não é redefinido para matrizes e seu resultado é bastante inútil. Geralmente, isso pode ser encontrado nas mensagens de diagnóstico .
int[] dim = {0, 0, 0}; ... return "Dim:" + dim + " SizeX:" + grid.length + " SizeY:" + grid[0].length + " SizeZ:"...
É difícil perceber o problema com os olhos, porque aqui dim.toString() implícito, mas a concatenação de strings delega a ele. Uma correção é sugerida imediatamente - envolva em Arrays.toString(dim) .
5. A coleção é lida, mas não preenchida
Isso também é frequentemente encontrado em uma base de código que não passa por testes constantes por um analisador estático. Aqui está um exemplo simples :
final Set<IBond> bondsToHydrogens = new HashSet<IBond>();
Obviamente, o preenchimento faltou. Os analisadores estáticos têm verificações mais simples que indicam uma variável não utilizada, mas a variável é usada aqui, portanto, elas são silenciosas. Precisamos de uma inspeção mais inteligente que saiba sobre coleções.
6. Pelo contrário: preenchemos, mas não lemos
Casos inversos também são possíveis. Aqui está um exemplo com uma matriz :
int[] tmp = new int[trueBits.length - 1]; System.arraycopy(trueBits, 0, tmp, 0, i); System.arraycopy(trueBits, i + 1, tmp, i, trueBits.length - i - 1);
A inspeção sabe que o terceiro argumento para o método arraycopy é usado apenas para gravar a matriz e, depois disso, a matriz não é usada. A julgar pela lógica do código, a linha trueBits = tmp; ignorada trueBits = tmp; .
7. Comparação de Inteiro por ==
Esse é um bug insidioso, porque pequenos valores de objetos Inteiros são armazenados em cache e tudo pode funcionar bem até que um dia o número exceda 127. Esse problema pode não ser evidente :
for (int a = 0; a < cliqueSize; a++) { for (int b = 0; b < vecSize; b += 3) { if (cliqueList.get(a) == compGraphNodes.get(b + 2)) { cliqueMapping.add(compGraphNodes.get(b)); cliqueMapping.add(compGraphNodes.get(b + 1)); } } }
Bem, parece que alguns objetos em algumas listas são comparados, talvez esteja tudo bem. É preciso ter cuidado para ver que essas listas contêm objetos do tipo Inteiro.
8. Duplicar no mapa
Nesta inspeção, uma imagem vale mais que mil palavras. Vê o erro ?
9. O resultado do método não é usado.
O resultado de alguns métodos é tolice não usar, o que a IDEA relata prontamente :
currentChars.trim();
Provavelmente, isso significava currentChars = currentChars.trim(); . Como as seqüências de caracteres em Java são imutáveis, se o resultado não for reatribuído, nada acontecerá. Também é encontrado, por exemplo , str.substring(2) .
A propósito, esta é uma inspeção bastante complicada. Além de uma lista pré-preparada de métodos, às vezes tentamos determinar automaticamente métodos cujo resultado vale a pena usar. Aqui, é necessária uma análise interprocedural, tanto no texto de origem quanto no código de bytes das bibliotecas. E tudo isso é feito em tempo real no processo de edição do código!
10. Ramos de switch inacessíveis
Como excluímos caracteres com um código maior que 128, os ramos \u2012-\u2212 inacessíveis. Parece que não valia a pena excluir.
11. Condição inatingível
Um problema absolutamente maravilhoso na cadeia de condições :
if (oxNum == 0) { if (hybrid.equals("sp3")) { ... } else if (hybrid.equals("sp2")) return 47; } else if (oxNum == 1 && hybrid.equals("sp3")) return 47; else if ((oxNum == 2 && hybrid.equals("sp3")) || (oxNum == 1 && hybrid.equals("sp2")) || (oxNum == 0 && hybrid.equals("sp")))
Na lógica condicional complexa, isso não é incomum: verificamos uma condição que não pode ser verdadeira, porque seu fragmento já foi verificado acima. Aqui temos um ramo separado oxNum == 0 , caso contrário, verificamos oxNum == 0 && hybrid.equals("sp") , que, é claro, não pode ser.
12. Nós escrevemos para uma matriz de comprimento zero
Às vezes, o IntelliJ IDEA notará se você gravar em uma matriz fora do seu tamanho :
Point3d points[] = new Point3d[0];
13. Verificando o comprimento após acessar o índice
Outro problema comum com o procedimento e novamente durante o tratamento de erros :
public void setParameters(Object[] params) throws CDKException { if (params.length > 1) { throw new CDKException("..."); } if (!(params[0] instanceof Integer)) {
No caso de uma matriz vazia, o autor do código queria sair silenciosamente, mas, devido à verificação, sairia, batendo com força em ArrayIndexOutOfBoundsException. Obviamente, a ordem de verificação está fora de ordem.
14. Verifique se há nulo após o acesso
E, novamente, a ordem das ações é violada, desta vez com nulo :
while (!line.startsWith("frame:") && input.ready() && line != null) { line = input.readLine(); logger.debug(lineNumber++ + ": ", line); }
A IDEA escreve essa line != null sempre verdadeiro. Acontece que a verificação é realmente redundante, mas aqui o código parece como se fosse nulo.
15. Disjunção em vez de conjunção
As pessoas frequentemente confundem os operadores lógicos AND e OR. O projeto CDK não é exceção :
if (rStereo != 4 || pStereo != 4 || rStereo != 3 || pStereo != 3) { ... }
Qualquer que seja o rStereo e o pStereo , é claro que eles não podem ser iguais a quatro e três ao mesmo tempo, portanto, essa condição é sempre verdadeira.
16. Novamente disjunção em vez de conjunção
Um erro semelhante , mas capturado por outra mensagem:
if (getFirstMapping() != null || !getFirstMapping().isEmpty()) { ... }
Podemos chegar ao lado direito apenas se getFirstMapping() retornou null , mas neste caso, temos a garantia de uma NullPointerException, sobre a qual a IDEA alerta. A propósito, aqui contamos com a estabilidade dos resultados do método getFirstMapping() . Às vezes usamos heurísticas, mas a estabilidade é diretamente analisada aqui. Como a classe é final, o método não pode ser substituído. return firstSolution.isEmpty() ? null : firstSolution IDEA verifica se seu corpo return firstSolution.isEmpty() ? null : firstSolution return firstSolution.isEmpty() ? null : firstSolution e determina que a estabilidade se resume à estabilidade do método Map#isEmpty , anteriormente anotado como estável.
17. Hierarquia de interfaces e instância de
Ao verificar se um objeto pertence a qualquer interface, não esqueça que as interfaces podem herdar uma da outra :
if (object instanceof IAtomContainer) { root = convertor.cdkAtomContainerToCMLMolecule((IAtomContainer) object); } else if (object instanceof ICrystal) { root = convertor.cdkCrystalToCMLMolecule((ICrystal) object); } ...
A interface ICrystal estende a interface IAtomContainer , de modo que o segundo ramo é obviamente inatingível: se um cristal chegar aqui, ele cairá no primeiro ramo.
18. Atravessando uma lista vazia
O autor deste código provavelmente não está muito familiarizado com a linguagem Java:
List<Integer> posNumList = new ArrayList<Integer>(size); for (int i = 0; i < posNumList.size(); i++) { posNumList.add(i, 0); }
O parâmetro size no ArrayList indica o tamanho inicial da matriz interna. Isso é usado para otimização, a fim de reduzir o número de alocações, se você souber com antecedência quantos itens colocar lá. No entanto, na verdade, os itens da lista não aparecem e o método size() retorna 0. Portanto, o próximo ciclo com uma tentativa de inicializar os itens da lista com zeros é completamente inútil.
19. Não se esqueça de inicializar os campos
O analisador verifica de maneira especial os construtores, levando em consideração os inicializadores de campo. Graças a isso, foi encontrado um erro :
public class IMatrix { public double[][] realmatrix; public double[][] imagmatrix; public int rows; public int columns; public IMatrix(Matrix m) { rows = m.rows; columns = m.columns; int i, j; for (i = 0; i < rows; i++) for (j = 0; j < columns; j++) { realmatrix[i][j] = m.matrix[i][j];
Apesar do fato de os campos serem públicos, ninguém aqui poderia defini-los e inicializá-los na frente do construtor. Portanto, a IDEA emite um aviso ousado de que o acesso a um elemento da matriz gera uma NullPointerException.
20. Não repita duas vezes
Condições repetidas também acontecem com frequência. Aqui está um exemplo :
if (commonAtomCount > vfMCSSize && commonAtomCount > vfMCSSize) { return true; }
Esses erros são insidiosos, porque você nunca sabe, a segunda condição é simplesmente supérflua ou o autor queria verificar outra coisa. Se isso não for corrigido imediatamente, pode ser difícil descobrir. Essa é outra razão pela qual a análise estática deve ser usada constantemente.
Eu relatei alguns desses erros no rastreador de erros do projeto . É curioso que, quando os autores do projeto corrigiram uma parte, eles mesmos usaram o analisador IntelliJ IDEA, encontraram outros problemas sobre os quais não escrevi e também começaram a corrigi-los . Eu acho que este é um bom sinal: os autores perceberam a importância da análise estática.