microsoft / linkcheckermd

A VSCode extension that check links in Markdown to ensure they are valid.
MIT License
16 stars 20 forks source link

Error message misuses `hasCountryCode` as an arraylike object #47

Closed mavaddat closed 1 year ago

mavaddat commented 3 years ago

Source: https://github.com/microsoft/linkcheckermd/blob/85f46f4978c0a3665306429d62a8f3036d198242/extension.ts#L195

The function isCountryCodeLink below expects to be able to use hasCountryCode as though it were an indexable object (like an array or list), but hasCountryCode is the function handle.

The line `Link ${link.address} contains a language reference: ${hasCountryCode[0]} ` uses the `hasCountryCode` as though it is indexable. However, `hasCountryCode` is a function. ```ts // Check for addresses that contain country codes function isCountryCodeLink(link: Link): Diagnostic { let countryCodeDiag=null; //If one was found... if(hasCountryCode(link.address)) { //Create the diagnostics object countryCodeDiag = createDiagnostic( DiagnosticSeverity.Warning, link.text, link.lineText, `Link ${link.address} contains a language reference: ${hasCountryCode[0]} ` ); } return countryCodeDiag; } function hasCountryCode(linkToCheck: string): boolean { //Regex for country-code let hasCountryCode = linkToCheck.match(/(.com|aka\.ms)\/[a-z]{2}\-[a-z]{2}\//); return hasCountryCode ? true : false; } ```

I guess the second function got refactored out of the first one and the developer forgot that the hasCountryCode moved to a different scope.

Perhaps a suitable correction would be to pass a reference that gets set to the country found like so: ```ts // Check for addresses that contain country codes function isCountryCodeLink(link: Link): Diagnostic { let countryCodeDiag=null; let countryCode= new Array(""); //If one was found... if(hasCountryCode(link.address, countryCode)) { //Create the diagnostics object countryCodeDiag = createDiagnostic( DiagnosticSeverity.Warning, link.text, link.lineText, `Link ${link.address} contains a language reference: ${countryCode[0]} ` ); } return countryCodeDiag; } function hasCountryCode(linkToCheck: string, countryCode?: Array): boolean { //Regex for country-code let hasCountryCode = linkToCheck.match(/(?:.com|aka\.ms)\/([a-z]{2}\-[a-z]{2})\//); if( countryCode !== undefined) { countryCode = hasCountryCode; } return hasCountryCode ? true : false; } ```
michaelgwelch commented 1 year ago

I think this is fixed by #50 which no longer attempts to output the country code. It just outputs the rule # LNK0001 instead. But I'm not sure that the fixed version has been released yet.

Nope: The version is from 2020:

image

And the PR is from this year.