[Spec] nifxml linter #74
Comments
|
Cases where this should NOT be allowed is you have Version comparisons in the vercond which means you're splitting checks on the Version across 3 attributes. Could you give an example for this, I'm not sure I fully grasp the intent. |
|
Exactly what I said: Splitting ver1="10.1.0.0" vercond="!((Version == 20.2.0.7) && (User Version 2 > 0))"Firstly, But ignoring the tokens for now, add in more version attributes and it becomes a confusing mess: ver1="10.1.0.0" ver2="20.5.0.0" vercond="!((Version == 20.2.0.7) && (User Version 2 > 0))"
ver1="10.1.0.0" ver2="20.5.0.0" userver="0" vercond="!((Version == 20.2.0.7) && (User Version 2 > 0))"So you now have 3+ attributes to deal with, all which have some check on Version etc.. Maybe those checks do not conflict but maybe they do. It should just be written as an expression. Once tokenized however, it's not too bad, but it should still be avoided: ver1="10.1.0.0" ver2="20.5.0.0" vercond="!#BS202#"So, as of right now, there are 44 members that the linter warns on for having Which is 44 instances, which is around 1/4th of all of the I may demote the |
First batch of fixed issues detected by the new linter. Also some slight cleanup, plus an interitance fix a Bethesda particle class.
|
Thank you for the clarification. |
For niftools#74 Implemented thanks to changes done in niftools#76 for differentiating integral and "countable" i.e. can be used for sizes. Marking types as boolean was also in niftools#76, yet there were no errors for this lint; all the `cond` references that were not complex expressions were already boolean types.
|
Concerning default strings as used as used in BSConnectPoint: How can a string be distinguished from any other type? Is it on the field type to handle that? |
While rewriting nifxml.py I have written some linting functions to catch issues that simple stuff like XSD and DTD cannot.
Error Levels
CRT= CriticalERR= ErrorWRN= WarningINF= InfoFor this issue I will refer to:
ver1asminverver2asmaxverGeneral Issues
CRTMissingnamefor XML tags:basic,compound,niobject,enum,bitflags,tokenCRTMissingidfor XML tags:versionMember-type Issues
Collision issues
ERRMember names that are also type names (compound, NiObject, enum, etc).ERRExpression attributes that are using type names.ERRCannot have bothonlyTandexcludeT(See [Spec] Additions, Removals, and Deprecations #76)WRNShould not havecondalongsideonlyT/excludeT.WRNMembers that have (minverormaxveretc.) andvercondvercondis present, no other version-specific attributes should exist as it spreads out the version conditions and reduces readability and maintainability.Versioncomparisons in thevercondwhich means you're splitting checks on theVersionacross 3 attributes, e.g.ver1="X" ver2="Y" vercond="Version != Z"Not Enough Information Issues
CRTMissingnameortypenameandtype.ERRtypethatis_templatebut emptytemplatestringIncorrect Information Issues
ERRnameortypedo not match identifier regexERRtemplatebut notypeor unexpectedtypetemplatefilled out.ERRtypethat should not have adefaultsetERRtypethat is Ref/Ptr buttemplateis not a NiObjecttemplatetype that is an NiObjectERRType inonlyT/excludeTmust be an NiObject and must inherit the member parent NiObject.ERRdefaultcontains a comma-separated list that is not parseable into a tuple of floats or ints.WRNdefaultcould not be parsed into an enumerated value, int, float, or tuple of int/floats.There are currently no defaults that evaluate to strings or non-numeric values. However, default strings could eventually be implemented for certain types so this will just be a warning until that time.BSConnectPoint.Redundant Information Issues
WRNMembers named "Unknown" that also have a descriptionWRNMembers that do not appear in any<version>Reference Issues
ERRExpression forvercondhas identifier that is not a global namevercondexpression references something other than the global version identifiers from the Header.WRNReferenced name does not always exist in some capacity for all of this Member's versionsWRNExpression forcond,arr1,arr2,arghas identifier that is not a local name (Compound)WRNExpression forcond,arr1,arr2,arghas identifier that is not a local name (NiObject)WRNExpression forcond,arr1,arr2,arghas identifier that is not a local or inherited name (NiObject)WRNReferenced member forcondis not a boolean typeWRNReferenced member forarr1is not an integral typeWRNReferenced member forarr2is not an integral type or an integral array typearr2is currently up for review, but as it currently stands, doing so is both valid and necessary (Strip Lengthsfor tri strips for example)All of these must also filter out various tokens like
#ARG#or#T#before doing checks.Duplicate Issues
ERRDuplicatenamethat appear in the same version. For NiObject, check inheritance.ERRDuplicatenamethat differ in castable type but do not have a suffix, regardless of version exclusivityuint. A ushort can fit inside a uint, so there is no problem with this naming collision so long as they are version exclusive. But a short cannot come before an int as the data member will be declared as a short, and any int value will be truncated when trying to store in that field.Enum-type Issues
ERROptionnameorprefixthat do not start with an alpha characterERRdefaultthat do not exist in the enum optionsWRNInconsistent use of prefix/no-prefix enum strings. Prefer prefix-less.WRNdefaultthat use integers instead of enumeration strings (excluding bitflags)I've mostly written these from violations actually found in nif.xml while writing the linter, so this list is going to keep expanding as I add more linting. I will post examples of all the errors I've caught whie developing it.
The text was updated successfully, but these errors were encountered: