image1.png

Xmage √©s una aplicaci√≥ client / servidor per reproduir m√†gia : La reuni√≥ (MTG). Xmage va comen√ßar a evolucionar a principis de 2010. Durant aquest temps es van publicar 182 llan√ßaments, un ex√®rcit de partidari sencer es va reunir i el projecte encara es desenvolupa activament. Una excel¬∑lent oportunitat per participar en el seu desenvolupament tamb√©! Per tant, avui, el PVS-Studio Unicorn comprovar√† la base del codi XMage, i qui sap, podria entrar en conflicte amb alg√ļ en combat.

En definitiva, el projecte

XMage ha estat desenvolupant activament durant 10 anys. El seu objectiu √©s crear una versi√≥ en l√≠nia gratu√Įta i joc de targeta de codi obert Magic: la reuni√≥ original.

  • Acc√©s a prop de 19.000 mapes √ļnics expedits durant els 20 anys de la hist√≤ria de MTG;
  • Control autom√†tic i aplicaci√≥ de totes les regles del joc existent;
  • ;
  • (AI);
  • (est√†ndard, modern, verge, ordre);
  • ,.
  • / ul>

    Vaig caure sobre l’obra d’estudiants de la Universitat de Tecnologia de Delft (mestre d’arquitectura de programari). Aix√≤ consistia que els nois van participar activament en projectes de codi obert, que havien de ser bastant complexos i desenvolupar-se activament. Durant un per√≠ode de vuit setmanes, els estudiants han estudiat el curs i els projectes de codi obert per entendre i descriure l’arquitectura del programari seleccionat.
    Així que això és tot. En aquest treball, els nois van analitzar el projecte XMage i un dels aspectes del seu treball era obtenir diverses mètriques utilitzant Sonarqube (nombre de línies de codi, complexitat ciclomàtica, duplicació de codi, olor de codi, errors, vulnerabilitats, etc.).
    La meva atenci√≥ va ser atreta pel fet que en el moment del 2018, l’an√†lisi de Sonarqube va mostrar 700 defectes (errors, vulnerabilitats) per 1000000 l√≠nies de codi.
    Despr√©s de buscar a la hist√≤ria dels col¬∑laboradors, vaig descobrir que des de l’informe rebut amb advert√®ncies, van fer una sol¬∑licitud de pull per corregir uns 30 defectes en la categoria “bloquejador” o “cr√≠tic”. No es coneix la resta de les advert√®ncies, per√≤ espero que no s’han perdut.
    Ha passat dos anys ja que i la base de codi ha augmentat unes 250.000 línies de codi: una bona raó per veure com passen les coses.

    Sobre l’an√†lisi

    Per a l’an√†lisi, he pres la versi√≥ XMage – 1.4.44V0.
    Tenia molta sort amb el projecte. La construcció de xmació amb Maven va resultar molt senzilla (com es va escriure en la documentació):

mvn clean install -DskipTests

No es necessitava més de mi. Guai?

La integració del connector PVS-Studio a Maven tampoc no té cap problema: tot és com en la documentació.
Despr√©s de l’an√†lisi, es van rebre els avisos 911, incloent 674 per advert√®ncies d’1 i 2 nivells de confian√ßa. Als efectes d’aquest article, no he tingut en compte els avisos de nivell 3 perqu√® normalment hi ha un alt percentatge de falsos positius. M’agradaria cridar l’atenci√≥ sobre el fet que quan utilitzeu un analitzador est√†tic en una batalla real, no es pot ignorar aquestes advert√®ncies perqu√® tamb√© poden indicar defectes significatius en el codi.
A més, no he tingut en compte les advertències de determinades normes sobre els motius que són millors per als que coneixen el projecte com jo:

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

A m√©s, diverses regles de diagn√≤stic han produ√Įt uns 20 falsos positius evidents del mateix tipus. Guardat a Todo! En conseq√ľ√®ncia, si restem tot, em van enviar prop de 190 positius per a la seva consideraci√≥.
En considerar els desencadenants, s’han identificat molts defectes menors del mateix tipus, que estaven relacionats amb la depuraci√≥ o una operaci√≥ de verificaci√≥ o sense sentit. A m√©s, molts positius es van associar amb una pe√ßa de codi molt estranya que es va demanar que es rebutgi.
En conseq√ľ√®ncia, per a aquest article, he identificat 11 regles de diagn√≤stic i he analitzat un dels disparadors m√©s interessants.
Fem una ullada al que va passar.

N1 ADVERT√ąNCIA

V6003 L’√ļs del model ‘si (targeta! = null) {…} m√©s si (targeta! = null) {…}’ ha estat detectat. Hi ha una probabilitat de pres√®ncia d’errors l√≤gica. TORRETREGEARHULK.Java (90), torrectalgeArhulk.java (102)

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

Tot √©s senzill aqu√≠: el cos de la segona instrucci√≥ condicional si (targeta! = Null) al Si-altre, si la construcci√≥ mai no s’executar√†, ja que el programa no arribar√† a aquest punt o targeta. = Null sempre estar√† equivocat.

avís N2

v6004 La instrucci√≥ “llavors” √©s equivalent a la “altra” instrucci√≥. AsotorFecTimpl.java (35), AsotorFecTimpl.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√ļ que s’ha produ√Įt en la meva pr√†ctica de verificaci√≥ de projectes de codi obert. Copiar enganxar? O trobo a faltar alguna cosa? Suposo que encara ha de retornar FALSE a la branca d’altres. PS Si hi ha alguna cosa, no hi ha cap trucada recursiva (….), ja que s√≥n m√®todes diferents. Trigger similar:

  • v6004 La instrucci√≥ “llavors” √©s equivalent a la declaraci√≥ “m√©s”. Guidisplayutil.java (194), guidisplayutil.java (198)
  • advert√®ncia N3

    v6007 expressi√≥ “filter.getMessage (). Tolowercase (local.English) .startswith (“cadascun”) sempre est√† malament. Setpowertoughnessafect.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();}

    Normes de diagn√≤stic Triggers v6007 s√≥n molt populars per a cada projecte que es verifica. Xmage no √©s una excepci√≥ (79 peces). Actuaci√≥ de la regla, en principi, tot est√† en el cas, per√≤ molts casos cauen en depuraci√≥, llavors en reasseguran√ßa, despr√©s en una altra cosa. En general, √©s millor que l’autor del codi supervisi aquests positius que per a mi.
    Aquest disparador, per√≤, √©s sens dubte un error. Depenent de l’inici de la l√≠nia Filter.GetMessage () a Sble text “A …” o “tenir …” s’afegeix. Per√≤ l’error √©s que els desenvolupadors verifiquen que la cadena comen√ßa amb una lletra maj√ļscula, despr√©s de convertir la mateixa cadena en min√ļscula abans d’aquest. Oops. Com a resultat, la l√≠nia afegida sempre ser√† “tenir …”. El resultat de la falla no √©s cr√≠tic, sin√≥ tamb√© desagradable: apareixer√† un text de text compost en algun lloc. Els punts positius que he trobat el m√©s interessant:

    • v6007 L’expressi√≥ “t.startswith (” – “) ‘√©s sempre falsa. Boostsourceeffect.java (103)
    • L’expressi√≥ v6007 ‘setnames.isimpty () √©s sempre falsa. DownloadPicTesservice.Java (300)
    • L’expressi√≥ v6007 ‘existentbucketname == null’ sempre est√† malament. S3uploader.java (23)
    • v6007 expressi√≥ ‘! Lastrule.endswith (“.”) Sempre √©s cert. Efectes.java (76)
    • L’expressi√≥ v6007 ‘subtypestoignore :: cont√©’ √©s sempre false. VerifycarddaTatest.java (893)
    • L’expressi√≥ v6007 ‘notystartedTabletables == 1’ √©s sempre falsa. Mageserverimpl.java (1330)

    N4 advertència

    v6008 null dereference de “savedspecialrares”. Dragonsmaze.java (230)

    L’analitzador es queixa de la derefer√®ncia de la refer√®ncia NULL SavedSpecialrarares quan l’execuci√≥ arriba al primer farciment de la col¬∑lecci√≥.
    El primer que em ve al cap √©s simplement confondre Savespecialrarars == Null amb SavedSpecialrares! = Nul. Per√≤ en aquest cas, el NPE pot oc√≥rrer en el constructor de Arraylist quan la col¬∑lecci√≥ es retorna pel m√®tode, ja que Savespecialrares == Sempre √©s possible. Corregiu el codi amb la primera soluci√≥ que em ve a la meva ment no √©s una bona opci√≥. Despr√©s d’una mica de comprensi√≥ del codi, vaig descobrir que S Adspecialrares es defineix immediatament per una col¬∑lecci√≥ buida quan es declara i no es reassigna en cap altre lloc. Aix√≤ ens explica quantes preguntes mai no seran zero, i la derefer√®ncia d’una refer√®ncia zero, l’analitzador de la qual adverteix, mai passar√†, perqu√® mai arribar√† a la col¬∑lecci√≥. Com a resultat, el m√®tode sempre retornar√† una col¬∑lecci√≥ buida.

    PS Per resoldre aquest problema, heu de substituir SavedSpecialrarars == NULL per SAVEDSpecialrarars.Ispty ().
    PPS Alas, jugant en Xmage, no podreu obtenir targetes rares especials per a la col·lecció del laberint del Drac.
    Un altre cas de dereference d’una refer√®ncia zero:

    • V6008 zero derefer√®ncia de “correspond√®ncia”. Tablecontroller.java (973)

    advertència n5

    operador v6012 “:”, sigui quina sigui la seva expressi√≥ condicional, nom√©s retorna un mateix valor “taula.getcreatetime ( ) ‘. Tablemanager.java (418), tablemanager.java (418)

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

    Aqu√≠ l’operador ternari ?: Retorna el mateix valor, independentment de la taula.GeteStartTtime == null. Crec que la finalitzaci√≥ del codi ha jugat una broma cruel al desenvolupador. Opci√≥ de correcci√≥:

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

    n6

    advertència
    v6026 Aquest valor ja est√† assignat a la variable “this.loseter “.Configurar-secreetypetargetefect.java (54)

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

    Cadena doble assignaci√≥. Sembla que el desenvolupador va ser una mica transportat amb les tecles de drecera i no ho va adonar. Per√≤ ja que l’efecte t√© un gran nombre d’√†rees, el fragment mereix ser centrat.

    ADVERT√ąNCIA N7

    V6036 El valor de l’opci√≥ “Selecciona” no inicialitzada s’utilitza. Session.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 l’av√≠s de l’analitzador, podem concloure que selection.get () pot llan√ßar un NuSCHELEMENT.
    Vegem més de prop el que està passant aquí.
    Si creieu que el comentari que l’usuari ja existeix, no s’elevar√† cap excepci√≥:

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

    en aquest cas, l’execuci√≥ del programa n ‘no introdu√Įu el cos d’instrucci√≥ condicional. I tot estar√† b√©. Per√≤ llavors sorgeix la pregunta: per qu√® necessitem un operador condicional amb una mena de l√≤gica complexa si mai no s’executa?

    Però, què passa si el comentari no és res?

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

    A continuaci√≥, l’execuci√≥ entra al cos de la instrucci√≥ condicional i recupera l’usuari a trav√©s de Geruserbyname (). La validesa de l’usuari es torna a comprovar, suggerint que la selecci√≥ no es pot inicialitzar. No hi ha cap altra branca per a aquest cas, que tamb√© conduir√† a un NosuchelementException a la l√≠nia de codi en q√ľesti√≥.

    N8 avís

    v6042 La compatibilitat expressi√≥ amb el tipus “A” est√† marcada, per√≤ es converteix en el tipus “B”. CheckboxList.java (586)
    iv , converteix expl√≠citament l’objecte en aquest tipus. Per aix√≤, el m√®tode Setmodel llan√ßar√† immediatament un ClasscastException quan arribi.
    El fitxer de CheckboxList.java s’ha afegit fa dos anys i aquest error persisteix en el codi des de llavors. Pel que sembla, no hi ha proves per a la configuraci√≥ incorrecta, no hi ha cap √ļs real d’aquest m√®tode amb tipus de tipus inadequats, de manera que viu.
    Si, de sobte, alg√ļ es connecta a aquest m√®tode i llegeix Javadoc, esperar√† un IllegalarGumentarException, no a ClasscastException … No crec que ning√ļ col¬∑locant deliberadament amb aquesta excepci√≥, per√≤ qui ho sap.
    Tenint en compte la documentació, el codi hauria de semblar així:

    Avís n9

    reproductor de refer√®ncia v6060 S’ha utilitzat abans de ser comprovat contra NULL. VIGeanIntuition.java (79), VIGeanIntuition.java (78)

    v6060 adverteix el desenvolupador Un objecte s’est√† accedint abans que no es comprovi per NULL. Els desencadenants d’aquesta regla es troben sovint en els articles sobre auditoria de projectes de codi obert: en general, la ra√≥ √©s el frac√†s de la refactoritzaci√≥ o la modificaci√≥ dels contractes de m√®todes. Si pareu atenci√≥ al m√®tode GetPlayer (), a continuaci√≥, tot es configurar√† immediatament:

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

    N10 avís

    V6072 dos fragments de codi similar s’han trobat. Pot ser que sigui una fallada d’escriptura i la variable “Playerb” s’hauria d’utilitzar en lloc de “Playera”. Subtypechangingeffexttest.java (162), subtipusThangingeffexttest.java (158), subtyphangingeffexttest.java (156), subtipus de subministrament (160) ava (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)); } }}

    haver vist que aquest error est√† a la p√†gina Proves, es pot devaluar immediatament la falla que es troba en el pensament: “B√©, s√≥n proves”. Si aquest √©s el cas, no estic d’acord amb tu. Despr√©s de tot, les proves tenen un paper bastant important en el desenvolupament (per√≤ no tan notable com la programaci√≥), i quan apareix una falla en una versi√≥, comencen immediatament a assenyalar les proves / testers. Per tant, les proves defectuoses s√≥n insostenibles. Per qu√® s√≥n necess√†ries aquestes proves? Per qu√® els recursos residuals?
    El m√®tode TestArcaneadAptygivetype () prova la targeta “Adaptaci√≥ arcana”. Cada jugador rep cartes en una zona de jocs espec√≠fica. I gr√†cies a la c√≤pia-pasta, la pastisseria t√© 2 cartes “SilverCoat Lion” id√®ntic al parc infantil “cementiri”, i jo JugadorBdun res ha passat. A m√©s, una mena de m√†gia i provar proves.
    Quan la prova arriba al reproductor “cementiri” a la concentraci√≥ actual, llavors l’execuci√≥ de la prova no entrava mai al bucle, perqu√® no hi havia res al “cementiri”. Aix√≤ √©s el que vaig descobrir amb el bon sistema vell.Out.Println () en iniciar la prova.
    Còpia-pasta corregida:

    ....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"); // <=....

    Després de canviar el codi, quan vaig executar la prova, la recerca de criatures en el cementiri de jugadors que va començar a treballar. AVE, SYSTEM.OUT.PRINGLN ()!
    La prova √©s verda abans i despr√©s de la correcci√≥, que √©s molta sort. Per√≤ en cas de modificacions que modifiquen la l√≤gica de l’execuci√≥ del programa, aquesta prova us far√† mal servei, informant-vos de l’√®xit fins i tot si hi ha errors.
    Same Copy-Stick:

    • V6072 S’han trobat dos fragments de codi similar. Pot ser que sigui una fallada d’escriptura i la variable “Playerb” s’hauria d’utilitzar en lloc de “Playera”. PintorsServantTest.Java (33), pintersservanttest.java (29), pintersservanttest.java (27), pintersservanttest.java (31)
    • v6072 S’han trobat dos fragments de codi similar. Pot ser que sigui una fallada d’escriptura i la variable “Playerb” s’hauria d’utilitzar en lloc de “Playera”. SubtypeNangingeffextTest.Java (32), subtipusEnvanzingefectesTestest.java (28), subtyphangingeffexttest.java (26), subtyphangingeffexttest.java (30)

    advertència N11

    V6086 Suspeced codi Formataci√≥. Probablement falta la paraula clau “m√©s”. 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√≤stic V6086 diagn√≤stic un incorrecte IF-One-Si formataci√≥, que implica l’omissi√≥ d’altres.
    Aquest extracte de codi il¬∑lustra aix√≤. En aquest cas, a causa de l’expressi√≥ de la devoluci√≥ nul¬∑la, una inexactitud en formataci√≥ no condueix a res, per√≤ √©s genial trobar aquests casos, perqu√® no √©s necessari.
    Penseu en un cas on l’omissi√≥ de l’altra pugui provocar un comportament inesperat:

    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;}

    ara, en el cas d’obj = nul, s’assignar√† l’objecte en q√ľesti√≥ Un valor, i els que falten, faran que l’objecte assignat recentment comprovi la cadena IF-Ole-IF, mentre que l’objecte se suposa que ha de retornar immediatament al m√®tode.

    Conclusió

    Comprovaci√≥ de xmage √©s un altre article que revela les capacitats dels analitzadors est√†tics moderns. En el desenvolupament modern, la seva necessitat nom√©s augmenta, ja que augmenta la complexitat del programari. I, independentment del nombre de versions, proves, comentaris de l’usuari: un error sempre trobar√† una falla per introduir la vostra base de codi. Per qu√® no afegir una altra barrera a la vostra defensa?
    A mesura que enteneu, els analitzadors estan subjectes a falsos positius (incloent PVS-Studio Java). Aix√≤ pot ser el resultat d’un defecte obvi i un codi massa conf√ļs (per desgr√†cia, l’analitzador no ho va entendre). Heu de tractar-los amb la comprensi√≥ i la subscripci√≥ immediatament sense dubtes, per√≤ mentre que els falsos positius estan esperant la seva correcci√≥, podeu utilitzar un dels m√®todes d’av√≠s.
    En conclusi√≥, personalment us suggereixo que “toqueu” l’analitzador descarregant-lo des del nostre lloc web.

    Si voleu compartir aquest article amb un p√ļblic de parla anglesa, utilitzeu el Enlla√ß de la traducci√≥: Maxim Stefanov. Verificaci√≥ del codi XMage i per qu√® no podreu obtenir les targetes rares especials de la col¬∑lecci√≥ del laberint del Drac.

    Leave a comment

    L'adreça electrònica no es publicarà. Els camps necessaris estan marcats amb *