A TADS3 module for implementing linters

Authoring TADS
1

Here’s a simple module that’s intended to make it easier to implement linters: linter github repo.

You declare an instance of the Linter class, putting your static analysis tests in the lint() method.

You can use Linter.error() to add an error, and Linter.warning() to add a warning:

              // Declare a linter.
              myLinter: Linter
                      lint() {
                              warning('this is an example warning');
                              error('this is an example error');
                      }
              ;

The lint() method will be called during preinit in debugging builds (when t3make is run with the -d flag).

By default the linter will throw an exception and exit at runtime if there are any errors, outputting all errors and warnings.

If compiled with -D WERROR the linter will treat all warnings as errors.

If the -d flag is not used when compiling the module will do nothing.

There’s also one utility method on Linter: superclassListIsABeforeB(obj, cls0, cls1). It checks the object obj to see if class cls0 occurs before class cls1 in the object’s superclass list.

I’ll probably add a few more utility methods to the base module, but it’s mostly designed as a framework for hanging bespoke tests off of.

6 Likes
2

Added a couple additional utility methods, all wrappers around basic forEachInstance loops:

  • isSingleton(cls) Returns boolean true if there’s only one instance of the class cls
  • checkForOrphans(cls) Checks all instances of the class cls and returns boolean true if any instance’s location is nil
  • testInstanceProp(cls, fn) Iterates over all instances of the class cls. If fn is a method or property, the value of that method or property on each instance will be checked. If fn is a function it will be called with each instance as its argument. The return value is boolean true if the return value of the check above is true for all instances, nil otherwise

The usage for the first two should be obvious. For the last one, it means that:

testInstanceProp(Foo, &someMethod)

…will test each instance of Foo and will return true if someMethod() returns true for all of them. Alternately,

testInstanceProp(Bar, { x: x.someProp == 5 })

…will test each instance of Bar and will return true if the
value of someProp is exactly 5 on all of them.

3 Likes
3

I am going to for sure adopt this when I get back (1 week to end of Comp!). Will create (assuming Night Elves don’t fix my shoes first…) checks for checkObjInConsultable(objCls, cnslt) and checkObjBlockedByConnector(objCls, trvlCnct) for some game-specific checking right out of the gate.

2 Likes
4

Another update. I’m fiddling around with some ways to declare checks.

The module now provides two more classes, LintClass and LintRule.

LintClass can be used to apply a check to every instance of a class. Example:

              myLinter: Linter;
              +LintClass @Foo
                      lintAction(obj) {
                              if(obj.foo == 'bar')
                                      warning('foo is bar');
                      }
              ;

This will iterate through all instance of Foo, calling lintAction() for each instance, with the argument being the instance itself.

LintClass provides error(), warning(), and info(), which just call the same methods on the parent Linter.

LintClass also provides a setFlag() method. It takes a single text literal as its argument. Flags can then be checked via LintRule. Example:

              myLinter: Linter;
              +LintClass @Foo
                      lintAction(obj) {
                              if(obj.foo == 'bar') {
                                      warning('foo is bar');
                                      setFlag('fooIsBar');
                              }
                      }
              ;
              +LintClass @Bar
                      lintAction(obj) {
                              if(obj.bar == 'foo') {
                                      warning('bar is foo');
                                      setFlag('barIsFoo');
                              }
                      }
              ;
              +LintRule [ 'fooIsBar', 'barIsFoo' ]
                      lintAction() {
                              error('foo and bar potentially reversed');
                      }
              ;

This will check every instance of Foo to see if any instance’s foo property is 'bar'. It will also check every instance of Bar to see if any instance’s bar property is 'foo'. On any match a warning will be added and a flag will be set. The LintRule will then match when both flags are set, and log an error.

Both LintClass and LintRule checks are run after the Linter’s lint() method is evaluated, so lint() can set flags to be checked by LintRules.

This is just intended to (hopefully) make it easier to write checks.

1 Like
5

Bump for a update I just pushed.

Main points:

  • Linters now have a per-instance active property. Default is true, and if active isn’t true the instance doesn’t run.
  • Added “common” linter rules; linter checks that are intended to be useful for games in general.
  • Added default linter rules. These are defined in Linter.defaultRules, set in linterDefaults.t. These are instances of linter rules to add all linters by default.

Right now there’s only one “common” linter rule. It checks all Thing instances for weakTokens that match elements of the noun list. For example, in an object declared like:

+deck: Thing '(of) (cards) deck/cards' 'deck of cards'
        "It's a deck of cards. "
;

…this will never match >X CARDS, for example, because the weak token (cards) will prevent the noun cards from matching this object.

In addition to the above, there are a couple of additional demos/tests of the module.

I’ll probably be back-porting some of my bespoke linter rules that I’ve developed for my own use into the linter module as I generally refactor my WIP.

3 Likes
6

Oooh, great reminder. Integrating this was on my todo list and got lost in the shuffle. I groan to think of the debug iterations it might have saved me.

Good add. This, in particular, bites me waaay more than it should after the first time I hit it. My brain simply refuses to acknowledge this and must be constantly reminded.

2 Likes
7

Yeah, the way weakTokens works has bit me more than once as well.

If anyone has any other common gotchas like that feel free to mention them for addition as checks. When I originally wrote the module the thing I really had in mind was writing checks for game/module-specific things instead of general TADS3/adv3 stuff (I think the original design case was the routeTable module, for making sure zone definitions are consistent).

But here I found myself writing another module-specific check for the weakTokens problem, so figured it probably makes sense to make it a generic/default check for all linters.

1 Like
8

Just pushed another minor feature update.

Main thing to notice is that now the logging methods on the linter/linter rule classes now take a second argument for extended help/information about whatever is being reported.

Messages are now numbered, and you can see the extended information by using >LINT WARNING [number], >LINT ERROR [number] and >LINT INFO [number] for warnings, errors, and informational messages (respectively).

I also added another default “generic” linter check. This one checks non-plural Thing instances for noun words that match plural words on other objects. This usually prevents the parser from considering the first object at all if it’s in the same scope as the second and the command only refers to the duplicate word. As an example, if you have a deck of cards declared deck/cards and a card object declared card*cards, then the deck object won’t be considered during noun resolution (all else being equal).

Example of transcript showing the new debugging action stuff (this is from the demo ./demo/src/commonTest.t in the repo):

====================
This is just a test of the stock "common" linter rules.
   errors: 0
   warnings: 2
warning 1: "deck of cards" has weakTokens that conflict with its noun list
warning 2: "deck of cards" has a non-plural noun that matches another object's
(cards) vocabulary
====================
Void
This is a featureless void.

You see a deck of cards here.

>lint warn 1
If an object declaration has weak tokens (words in the vocabulary in
parentheses) this will prevent the parser from matching that word as a noun.

The linter rule complains when it notices that an object has a weak token that
matches a noun on the same object, because that usually means one or the other
shouldn't be part of the object's vocabulary.

>

Mostly added this just to let me write notes to my future self in my linter code.

2 Likes
9

Another little update.

First bit is a new linter rule nameAsOtherLinter that checks for a couple common (for me, anyway) problems with NameAsOther:

  • warns if NameAsOther comes after Thing in the object’s superclass list
  • warns if targetObj is nil and targetObject has a value

Second bit is that the lint checks now run during init instead of preinit. This is entirely because some checks fail if preinit hasn’t been completed. I might work out some way of allowing rules to specify when they should be run, but right now I don’t have any cases where running in init breaks them, so everything’s in init for now.

1 Like
10

Another default lint check.

This one is isEquivalentLinter for checking Things with isEquivalent = true.

Specifically it looks for Thing instances with isEquivalent set and the same equivalenceKey but different vocabulary set in cmdDict.

For example, if you declare:

widget1: Thing '(very) (unremarkable) widget' 'widget'
        "An unremarkable widget. "
        isEquivalent = true
;
widget2: Thing '(unremarkable) widget' 'widget'
        "An unremarkable widget. "
        isEquivalent = true
;

That will be listed as:

You see two widgets here.

…but the player can nevertheless target one via the vocabulary differences:

>take very unremarkable widget
Taken.

>take very unremarkable widget
You are already carrying the widget.

With the isEquivalentLinter rule enabled (which it is by default), then the linter will report something like:

warning 3: Equivalent objects with equivalenceKey "widget" have varying vocabulary

And doing a >LINT WARN 3 to see the details:

>lint warn 3
Objects with isEquivalent = true and the same equivalenceKey are usually
intended to be indistinguishable from each other.  This warning means that the
linter found otherwise equivalent objects that have vocabulary differences.    

The equivalenceKey is "widget", and the differences are:

   adjective: very
2 Likes