Image1.png

XMage es una aplicaci√≥n de cliente / servidor para reproducir magia : La reuni√≥n (MTG). XMage comenz√≥ a evolucionar a principios de 2010. Durante este tiempo, se publicaron 182 comunicados, se reuni√≥ un Ej√©rcito de apoyo completo y el proyecto a√ļn se desarrolla activamente. ¬°Una excelente oportunidad para participar en su desarrollo tambi√©n! Por lo tanto, hoy, el PVS-Studio Unicorn registrar√° la base de c√≥digo XMage, y qui√©n sabe, podr√≠a entrar en conflicto con alguien en combate.

En resumen en el proyecto

XMage se ha desarrollado activamente durante 10 a√Īos. Su objetivo es crear una versi√≥n gratuita en l√≠nea y un juego de cartas de c√≥digo de c√≥digo abierto Magia: la reuni√≥n original.

  • Acceso a aproximadamente 19,000 mapas √ļnicos emitidos durante los 20 a√Īos de historia de MTG;
  • Control autom√°tico y aplicaci√≥n de todas las reglas del juego existente;
  • ;
  • (AI);
  • (est√°ndar, moderno, vintage, orden);
  • ,.

Cayé en el trabajo de los estudiantes de la Universidad de Tecnología de Delft (Maestro de Arquitectura de Software). Esto consistió en que los hombres tomaron una parte activa en proyectos de código abierto, que tuvieron que ser bastante complejos y desarrollarse activamente. Durante un período de ocho semanas, los estudiantes han estudiado el curso y los proyectos de código abierto para comprender y describir la arquitectura del software seleccionado.
as√≠ es todo. En este trabajo, los hombres analizaron el proyecto XMAGE, y uno de los aspectos de su trabajo fue obtener varias m√©tricas usando Sonarqube (n√ļmero de l√≠neas de c√≥digo, complejidad ciclom√°tica, duplicaci√≥n de c√≥digo, olor de c√≥digo, errores, vulnerabilidades, etc.).
Mi atención se siente atraído por el hecho de que en el momento de 2018, el análisis de Sonarqube mostró 700 defectos (errores, vulnerabilidades) para 1000000 líneas de código.
Despu√©s de haber buscado en la historia de los contribuyentes, descubr√≠ que, desde el informe recibido con advertencias, hicieron una solicitud de extracci√≥n para corregir alrededor de 30 defectos en la categor√≠a ¬ęBloqueador¬Ľ o ¬ęcr√≠tico¬Ľ. ¬ŅQu√© pasa con el resto de las advertencias no se conoce, pero espero que no se hayan perdido?
Ha pasado 2 a√Īos desde que y la base de c√≥digos ha aumentado en aproximadamente 250,000 l√≠neas de c√≥digo, una buena raz√≥n para ver c√≥mo est√°n sucediendo las cosas.

Acerca del an√°lisis

Para análisis, tomé la versión XMage Р1.4.44v0.
tuve mucha suerte con el proyecto. Edificio Xmation con Maven resultó muy simple (como se escribió en la documentación):

mvn clean install -DskipTests

Nada m√°s se requer√≠a de m√≠. ¬ŅFresco?
La integraci√≥n del plugin de PVS-Studio en Maven tampoco tiene ning√ļn problema: todo es como en la documentaci√≥n.
Después del análisis, se recibieron 911 advertencias, incluyendo 674 para advertencias de 1 y 2 niveles de confianza. A los efectos de este artículo, no he tenido en cuenta las advertencias de nivel 3 porque generalmente hay un alto porcentaje de falsos positivos. Me gustaría llamar su atención sobre el hecho de que cuando usa un analizador estático en una batalla real, no puede ignorar tales advertencias porque también pueden indicar defectos significativos en el código. Además, no tuve en cuenta las advertencias de ciertas reglas sobre los motivos que son mejor tenidos en cuenta por aquellos que conocen el proyecto como yo:

  • v6022, /. 336.
  • v6014 ,,,, 73.
  • v6021 ,,. 36.
  • v6048 ,,. 17.

Además, varias reglas de diagnóstico han producido alrededor de 20 falsos positivos obvios del mismo tipo. ¡Guardado en todo!
En consecuencia, si restamos todo, se me enviaron unos 190 positivos para su consideración.
Cuando se considere desencadenantes, se han identificado muchos defectos menores del mismo tipo, que estaban relacionados con la depuraci√≥n o una operaci√≥n de verificaci√≥n o sin sentido. Adem√°s, muchos aspectos positivos se asociaron con una pieza de c√≥digo muy extra√Īa que se le pidi√≥ que se rechazara. En consecuencia, para este art√≠culo, he identificado 11 reglas de diagn√≥stico y analizaron uno de los desencadenantes m√°s interesantes.
eche un vistazo a lo que sucedió.

N1 Advertencia

V6003 ¬°El uso del modelo ‘IF (CARD! = NULL) {…} OF (¬°la tarjeta! = null) {…}’ ha sido detectado. Hay una probabilidad de presencia de error l√≥gica. TorrentialGearHulk.Java (90), torrentialgearhulk.java (102)

@Overridepublic boolean apply(Game game, Ability source) { .... Card card = game.getCard(....); if (card != null) { .... } else if (card != null) { .... } ....}

Todo es simple aquí: el cuerpo de la segunda instrucción condicional si (Tarjeta! = NULL) en el Si el otro, si la construcción nunca será ejecutada, ¡ya que el programa no alcanzará este punto, o la tarjeta! = NULL siempre estará equivocado.

N2 Advertencia

V6004 La instrucci√≥n ¬ęLUEGO¬Ľ es equivalente a la instrucci√≥n ¬ęde lo contrario¬Ľ. Athougheffectimpl.java (35), asthoughefectimpl.java (37)

@Overridepublic boolean applies(....) { // affectedControllerId = player to check if (getAsThoughEffectType().equals(AsThoughEffectType.LOOK_AT_FACE_DOWN)) { return applies(objectId, source, playerId, game); } else { return applies(objectId, source, playerId, game); }}

Un error com√ļn que se ha producido en mi pr√°ctica de verificaci√≥n de proyectos de c√≥digo abierto. ¬ŅCopiar pegar? O echo de menos algo? Supongo que todav√≠a tienes que volver falso a la rama m√°s. PDS Si es algo, no hay llamada recursiva (…), porque son m√©todos diferentes. Disparador similar:

  • V6004 La instrucci√≥n ¬ęEntonces¬Ľ es equivalente a la declaraci√≥n ¬ęde lo contrario¬Ľ. Guidisplayutil.java (194), guidisplayutil.java (198)
  • Advertencia N3

    V6007 Expression ‘filter.getMessage (). ToloWerCase (local.English) .ststswith (¬ęcada uno¬Ľ) ‘siempre est√° equivocado. Setpowertoughnessffect.java (107)

    @Overridepublic String getText(Mode mode) { StringBuilder sb = new StringBuilder(); .... if (filter.getMessage().toLowerCase(Locale.ENGLISH).startsWith("Each ")) { sb.append(" has base power and toughness "); } else { sb.append(" have base power and toughness "); } .... return sb.toString();}

    Reglas de diagnóstico Los disparadores V6007 son muy populares para cada proyecto que se está verificando. Xmage no es una excepción (79 piezas). Actuación de la regla, en principio, todo está en el caso, pero muchos casos caen en la depuración, luego en el reaseguro, luego en otra cosa. En general, es mejor que el autor del Código supervise tales positivos que para mí.
    Este gatillo, sin embargo, es definitivamente un error. Dependiendo del inicio de la l√≠nea Filter.getMessage () al texto de slebe ¬ęA …¬Ľ o ¬ęTener …¬Ľ se agrega. Pero el error es que los desarrolladores verifican que la cadena comience con una letra may√ļscula, despu√©s de convertir la misma cadena en min√ļsculas antes de eso. UPS. Como resultado, la l√≠nea a√Īadida siempre ser√° ¬ętener …¬Ľ. El resultado de la falla no es cr√≠tico, sino tambi√©n desagradable: un analfabeto de texto compuesto aparecer√° en alg√ļn lugar. Los puntos positivos que encontr√© lo m√°s interesante:

    • v6007 la expresi√≥n ‘t.sartswith (¬ę-¬ę)’ es siempre falso. Boostsourceeeffect.java (103)
    • La expresi√≥n V6007 ‘setnames.Isempty () siempre es falsa. DescargarPictureService.Java (300)
    • La expresi√≥n V6007 ‘existenteBucketName == NULL’ siempre est√° equivocado. S3uploader.java (23)
    • v6007 expresi√≥n ‘! Lastrula.endswith (¬ę¬Ľ) es siempre cierta. Effects.Java (76)
    • La expresi√≥n V6007 ‘subtipoPestoignore :: Contiene’ siempre es falso. Verifycarddatatest.java (893)
    • La expresi√≥n V6007 ‘NO SISTETTETTABLES == 1’ es siempre falso. Maageserverimpl.java (1330)

    n4 advertencia

    V6008 Null Dereference de ¬ęSavedspecialRares¬Ľ. Dragonsmoze.java (230)

    public final class DragonsMaze extends ExpansionSet { .... private List<CardInfo> savedSpecialRares = new ArrayList<>(); .... @Override public List<CardInfo> getSpecialRare() { if (savedSpecialRares == null) { // <= CardCriteria criteria = new CardCriteria(); criteria.setCodes("GTC").name("Breeding Pool"); savedSpecialRares.addAll(....); // <= criteria = new CardCriteria(); criteria.setCodes("GTC").name("Godless Shrine"); savedSpecialRares.addAll(....); .... } return new ArrayList<>(savedSpecialRares); }}

    El analizador se queja de la deferencia de la referencia NULL SavedSpecialares cuando la ejecución llega al primer llenado de la colección.
    Lo primero que viene a mi mente es simplemente confundir a SavesPeAlrares == NULL con SavedspecialRares! = Nulo. Pero en tal caso, la NPE puede ocurrir en el constructor de Arraylist cuando la colecci√≥n es devuelta por el m√©todo, porque SavespecialRares == Null es siempre posible. Corrija el c√≥digo con la primera soluci√≥n que viene a mi mente no es una buena opci√≥n. Despu√©s de un poco de entendimiento del C√≥digo, descubr√≠ que los l√°rminos de Adspecialr√°s se definen inmediatamente por una recolecci√≥n vac√≠a cuando se declara y no se realiza en ning√ļn otro lugar. Esto nos dice cu√°ntas preguntas nunca ser√° cero, y la deferencia de una referencia cero, cuyo analizador advierte, nunca suceder√°, ya que nunca llegar√° a la colecci√≥n. Como resultado, el m√©todo siempre devolver√° una colecci√≥n vac√≠a.
    PS Para resolver este problema, debe reemplazar a SavedSpecialRares == NULL por SavedSpecialRares.Isempty ().
    PPS ALAS, jugando en XMage, no podrá obtener tarjetas raras especiales para la colección del laberinto del dragón.
    Otro caso de dereferencia de una referencia cero:

    • v6008 cero Dereferencia de ¬ęcorrespondencia¬Ľ. TableController.Java (973)

    ADVERTENCIA N5

    V6012 Operador ‘?:’, Cualquiera que sea su expresi√≥n condicional, siempre devuelva solo un mismo valor ‘table.getcreatetime ( ) ‘. Tablemanager.java (418), tablemanager.java (418)

    private void checkTableHealthState() { .... logger.debug(.... + formatter.format(table.getStartTime() == null ? table.getCreateTime() : table.getCreateTime()) + ....); ....}

    ¡Aquí está el operador de Ternary?: Devuelve el mismo valor independientemente de la Tabla.GetStarttime Condición () == nulo. Creo que la finalización del Código ha jugado una broma cruel para el desarrollador. Opción de corrección:

    private void checkTableHealthState() { .... logger.debug(.... + formatter.format(table.getStartTime() == null ? table.getCreateTime() : table.getStartTime()) + ....); ....}

    N6

    Advertencia
    V6026 Este valor ya est√° asignado a la variable ¬ęEsto.LoseOtros ¬ę.Convertirse enETETYPETARGETEFECT.JAVA (54)

    publicBecomesCreatureTypeTargetEffect(final BecomesCreatureTypeTargetEffect effect) { super(effect); this.subtypes.addAll(effect.subtypes); this.loseOther = effect.loseOther; this.loseOther = effect.loseOther;}

    Cadena de doble asignación. Parece que el desarrollador estaba un poco arrastrado con las teclas de acceso directo y no lo notó. Pero como el efecto tiene una gran cantidad de áreas, el fragmento merece estar enfocado.

    N7 ADVERTENCIA

    V6036 Se utiliza el valor de la opci√≥n ‘Seleccionar’ no inicializada. SESIONE.JAVA (227)

    public String connectUserHandling(String userName, String password){ .... if (!selectUser.isPresent()) { // user already exists selectUser = UserManager.instance.getUserByName(userName); if (selectUser.isPresent()) { User user = selectUser.get(); .... } } User user = selectUser.get(); // <= ....}

    de la advertencia del analizador, podemos concluir que selection.get () puede lanzar un NOSUCHELEMENTO.
    Veamos más de cerca lo que está sucediendo aquí. Si cree que el comentario que ya existe el usuario, no se levantará ninguna excepción:

    ....if (!selectUser.isPresent()) { // user already exists ....}User user = selectUser.get()....

    En este caso, la ejecuci√≥n del programa n ‘no ingrese el Cuerpo de instrucci√≥n condicional. Y todo estar√° bien. Pero entonces surge la pregunta: ¬Ņpor qu√© necesitamos un operador condicional con un tipo de l√≥gica compleja si nunca se ejecuta?
    ¬ŅPero qu√© pasa si el comentario no es nada?

    ....if (!selectUser.isPresent()) { // user already exists selectUser = UserManager.instance.getUserByName(userName); if (selectUser.isPresent()) { .... }}User user = selectUser.get(); // <=....

    , la ejecución entra en el cuerpo de la instrucción condicional y recupera al usuario a través de GeruserbyName (). La validez del usuario se verifica de nuevo, sugiriendo que la selección puede no ser inicializada. No hay otra sucursal para este caso, que también conducirá a una excepción nosuchelementException en la línea de código en cuestión.

    N8 Advertencia

    V6042 La compatibilidad de la expresi√≥n con el tipo ¬ęA¬Ľ se verifica, pero se convierte en el tipo ¬ęB¬Ľ. Checkboxlist.java (586)

    /** * sets the model - must be an instance of CheckBoxListModel * * @param model the model to use * @throws IllegalArgumentException if the model is not an instance of * CheckBoxListModel * @see CheckBoxListModel */@Overridepublic void setModel(ListModel model) { if (!(model instanceof CheckBoxListModel)) { if (model instanceof javax.swing.DefaultListModel) { super.setModel((CheckBoxListModel)model); // <= } else { throw new IllegalArgumentException( "Model must be an instance of CheckBoxListModel!"); } } else { super.setModel(model); }}

    El autor del c√≥digo se confunde sobre algo aqu√≠: Primero, se asegura de que el modelo N ‘no sea una lista de verificaci√≥n, entonces , convierte expl√≠citamente el objeto en este tipo. Debido a esto, el m√©todo de SetModel iniciar√° inmediatamente una ClasscastException cuando llegue all√≠.

    El archivo CheckBoxList.java se ha agregado hace 2 a√Īos y este error persiste en el c√≥digo desde entonces. Aparentemente, no hay pruebas para configuraciones incorrectas, no hay un uso real de este m√©todo con tipos de tipos inapropiados, por lo que vive. De repente, si de repente, alguien se conecta a este m√©todo y lee a Javadoc, esperar√° una excepci√≥n ilegalAgumentException, no una ClassCastException … No creo que nadie se coloque deliberadamente con esta excepci√≥n, pero qui√©n sabe.
    Dada la documentación, el código debería ser más probable que se vea así:

    Advertencia N9

    V6060 Referencia ‘Jugador’ ‘Se us√≥ antes de ser revisado contra NULL. VIGEANINTUITION.JAVA (79), VIGEANINTUITION.JAVA (78)

    V6060 advierte al desarrollador que un objeto se está accediendo antes de que no se revise por NULL. Los desencadenantes de esta regla se encuentran a menudo en los artículos sobre auditoría del proyecto de código abierto: en general, la razón es la falla de la refactorización o la modificación de los contratos para los métodos. Si presta atención al método GetPlayer (), entonces todo se configurará de inmediato:

    // Result must be checked for null.// Possible errors search pattern: (\S*) = game.getPlayer.+\n(?!.+\1 != null)Player getPlayer(UUID playerId);

    N10 Advertencia

    v6072 Dos fragmentos de c√≥digo similar se han encontrado. Esto puede ser una falla de mecanograf√≠a y la variable ¬ęjugador¬Ľ debe usarse en lugar de ¬ęPLAYERA¬Ľ. SubtipoCechangingeFectstest.Java (162), subtipoPechangingeFectstst.Java (158), subtipoPechangingeFectstst.Java (156), subtipoPechangingeFectsTest.Java (160)

    @Testpublic void testArcaneAdaptationGiveType() { addCard(Zone.HAND, playerA, "Arcane Adaptation", 1); // Enchantment {2}{U} addCard(Zone.BATTLEFIELD, playerA, "Island", 3); addCard(Zone.HAND, playerA, "Silvercoat Lion"); addCard(Zone.BATTLEFIELD, playerA, "Silvercoat Lion"); addCard(Zone.GRAVEYARD, playerA, "Silvercoat Lion"); // <= addCard(Zone.HAND, playerB, "Silvercoat Lion"); addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion"); addCard(Zone.GRAVEYARD, playerA, "Silvercoat Lion"); // <= .... for (Card card : playerB.getGraveyard().getCards(currentGame)) { if (card.isCreature()) { Assert.assertEquals(card.getName() + " should not have ORC type", false, card.getSubtype(currentGame).contains(SubType.ORC)); Assert.assertEquals(card.getName() + " should have CAT type", true, card.getSubtype(currentGame).contains(SubType.CAT)); } }}

    Habiendo visto este error en el Pruebas, puede devaluar inmediatamente la falla que se encuentra en el pensamiento: ¬ęBueno, estas son pruebas¬Ľ. Si este es el caso, no estoy de acuerdo contigo. Despu√©s de todo, las pruebas desempe√Īan un papel bastante importante en el desarrollo (pero no tan notable como la programaci√≥n), y cuando aparece una falla en una versi√≥n, inmediatamente comienzan a se√Īalar las pruebas / probadores. Por lo tanto, las pruebas defectuosas son insostenibles. ¬ŅPor qu√© son necesarias tales pruebas? ¬ŅPor qu√© los recursos residuos en ellos?

    El m√©todo TestArcaneAptyGiveType () prueba la tarjeta ¬ęAdaptaci√≥n Arcana¬Ľ. Cada jugador recibe cartas en un √°rea de juego espec√≠fica. Y gracias a la Pasta de Copia, Playera obtuvo tarjetas de 2 ¬ęSilverCoat Lion¬Ľ id√©ntelos en el √°rea de juegos ¬ęcementerios¬Ľ, y jugadoresbdun no ha sucedido nada. Adem√°s, una especie de magia y prueba las pruebas.
    Cuando la prueba llega al PlayerB ¬ęCementerio¬Ľ en el rally actual, entonces la ejecuci√≥n de la prueba nunca ingresa al bucle, porque no hab√≠a nada en el ¬ęcementerio¬Ľ. Esto es lo que descubr√≠ con el buen sistema antiguo.Out.Println () al iniciar la prueba.
    Copie-Paste corregido:

    ....addCard(Zone.HAND, playerA, "Silvercoat Lion");addCard(Zone.BATTLEFIELD, playerA, "Silvercoat Lion");addCard(Zone.GRAVEYARD, playerA, "Silvercoat Lion"); // <=addCard(Zone.HAND, playerB, "Silvercoat Lion");addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion");addCard(Zone.GRAVEYARD, playerB, "Silvercoat Lion"); // <=....

    Despu√©s de cambiar el c√≥digo, cuando ejecu√≠ la prueba, la b√ļsqueda de criaturas en el Cementerio del PlayerB A comenz√≥ a funcionar. AVE, SYSTEM.OUT.PRINTLN ()!
    La prueba es verde antes y después de la corrección, que es mucha suerte. Pero en caso de modificaciones que modifiquen la lógica de la ejecución del programa, dicha prueba lo hará un mal servicio, informándole el éxito, incluso si hay errores.
    mismo Copy-Stick en otra parte:

    • v6072 Se han encontrado dos fragmentos de c√≥digo similar. Esto puede ser una falla de mecanograf√≠a y la variable ¬ęjugador¬Ľ debe usarse en lugar de ¬ęPLAYERA¬Ľ. Pintersservanttest.java (33), pintersservanttest.java (29), pintersservanttest.java (27), pintersservanttest.java (31)
    • V6072 Se han encontrado dos fragmentos de c√≥digo similar. Esto puede ser una falla de mecanograf√≠a y la variable ¬ęjugador¬Ľ debe usarse en lugar de ¬ęPLAYERA¬Ľ. SubtipoChangingEffectStest.Java (32), subtipoPechangingeFectsTest.Java (28), subtipoPechangingeFectsTest.Java (26), subtipoPechangingeFectstst.Java (30)

    ADVERTENCIA N11

    V6086 Sospecha de c√≥digo Formato. La palabra clave ¬ęotra cosa¬Ľ probablemente falte. Deckimporter.Java (23)

    public static DeckImporter getDeckImporter(String file) { if (file == null) { return null; } if (file.toLowerCase(Locale.ENGLISH).endsWith("dec")) { // <= return new DecDeckImporter(); } else if (file.toLowerCase(Locale.ENGLISH).endsWith("mwdeck")) { return new MWSDeckImporter(); } else if (file.toLowerCase(Locale.ENGLISH).endsWith("txt")) { return new TxtDeckImporter(haveSideboardSection(file)); } .... else { return null; }}

    La regla de diagnóstico V6086 Diagnóstico Unificio incorrecto if -se-siificado, que involucra la omisión de lo demás.
    Este extracto de código ilustra eso. En este caso, debido a la expresión del retorno nulo, una inexactitud en el formateo no conduce a nada, pero es genial encontrar estos casos, porque no es necesario.
    Considere un caso en el que la omisión del otro puede llevar a un comportamiento inesperado:

    public SomeType smtMethod(SomeType obj) { .... if (obj == null) { obj = getNewObject(); } if (obj.isSomeObject()) { // some logic } else if (obj.isOtherSomething()) { obj = calulateNewObject(obj); // some logic } .... else { // some logic } return obj;}

    , en el caso de obj = nulo, se le asignará el objeto en cuestión Un valor, y el más que faltan, hará que el objeto recién asignado revise a lo largo de la cadena si el objeto, mientras se suponía que el objeto volviera a regresar al método.

    Conclusión

    El control de XMage es otro art√≠culo que revela las capacidades de los analizadores est√°ticos modernos. En el desarrollo moderno, su necesidad solo aumenta, a medida que aumenta la complejidad del software. Y independientemente de la cantidad de versiones, pruebas, comentarios del usuario: un error siempre encontrar√° un error para ingresar su base de c√≥digo. Entonces, ¬Ņpor qu√© no agregar otra barrera a tu defensa?
    A medida que entienda, los analizadores están sujetos a falsos positivos (incluyendo PVS-Studio Java). Este puede ser el resultado de una falla obvia y un código demasiado confuso (ALAS, el analizador no lo entendió). Tienes que tratarlos con la comprensión y cancelar la suscripción inmediatamente sin dudarlo, pero si bien los falsos positivos están esperando su corrección, puede usar uno de los métodos de advertencia.
    En conclusi√≥n, personalmente le sugiero que ¬ętoque¬Ľ al analizador descarg√°ndolo desde nuestro sitio web.

    Si desea compartir este artículo con una audiencia de habla inglesa, use el Enlace de la traducción: Maxim Stefanov. Verificando el código XMage y por qué no podrá obtener las tarjetas raras especiales de la colección del laberinto del Dragón.

    Dejar un comentario

    Tu dirección de correo electrónico no será publicada. Los campos obligatorios están marcados con *