react-a11y-image-button-has-alt toLowerCase error throws

[dawsbot]

Only upon enabling "react-a11y-image-button-has-alt" with a value of true in our tslint.json do we begin seeing this issue. Can verify that other rules from tslint-microsoft-contrib are passing successfully in our codebase.

> tslint --project tsconfig.json

TypeError: Cannot read property 'toLowerCase' of undefined
    at ReactA11yImageButtonHasAltWalker.validateOpeningElement (/Users/Dawson/code/pland-ui/client/node_modules/tslint-microsoft-contrib/reactA11yImageButtonHasAltRule.js:66:77)

---

sist output:

node

npm -v: 5.4.0
node --version: v8.4.0

shell

uname: Darwin
echo $SHELL: /bin/zsh
echo $TERM: xterm-256color-italic
echo $TERM_PROGRAM: iTerm.app

Time created: Mon Aug 28 2017 16:55:50 GMT-0700 (PDT)

[willisplummer]

I've been seeing this too:

sist output:

shell

uname: Darwin
echo $SHELL: /bin/bash
echo $TERM: xterm-256color
echo $TERM_PROGRAM: vscode

node

npm -v: 3.10.8
node --version: v6.9.1

ruby

ruby --version: ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-darwin16]

go

go --version: error, return code 127

Time created: Fri Sep 01 2017 07:59:54 GMT-0400 (EDT)

[willisplummer]

okay it looks like this is the code responsible for the error:

    <img
      alt={caption}
      className="w100p"
      role={caption ? undefined : 'presentation'}
      src={src}
    />

when i removed the role attribute the error went away

[dawsbot]

@willisplummer perhaps this rule is calling toLowerCase on nullable data-types like undefined as well? I suppose a good first step would be to add a failing test to this code-base then work to fix it. TDD AF 👌

[willisplummer]

https://github.com/Microsoft/tslint-microsoft-contrib/blob/master/src/reactA11yImageButtonHasAltRule.ts#L56-L60

seems like the typeAttribute truthy check ought to prevent it from reaching toLowerCase with an undefined value.

FWIW I was a little confused. I actually arrived at this issue because I was seeing the same error from the reactA11yImgHasAltRule, which makes similar truthiness checks here.

[dawsbot]

@willisplummer I tried adding a failing test of your img example with no success. Perhaps you'll have different luck with this at https://github.com/Microsoft/tslint-microsoft-contrib/blob/master/src/tests/ReactA11yImageButtonHasAltRuleTests.ts#L21

Here's the simplified component I ran the tests with. That should cause a failure with the ReactA11yImageButtonHasAltRule rule, right?

        it('when img element has non-string alt text.', (): void => {
            const script: string = `
                import React = require('react');
                const altText = 3;
                const a = <input type='image' alt={altText} />;
            `;

            TestHelper.assertNoViolation(ruleName, script);
        });

[dawsbot]

Poked around a bunch, not happy with my abilities to debug. Would <3 help from someone at Microsoft on this one.

Pretty confident that replacing return undefined to return '' would fix the toLowerCase issue because you can actually call toLowerCase on undefined. But to do this proper TDD style I wanted to create a failing test first.