In certain cases, rules should fail if they don't get to check anything

[alrra]

A problem with rules passing even when they didn't get to check anything, is that, in certain cases, that will produce false results when we start to compare scans (i.e. "70% of the sites pass this rule", but did they? or did the 70% just not have the thing the rule checks for?

There are 2 cases that I can think of right now:

1. What the rule checks is mandatory

   These are the types of rules that enforce certain things to be used in a certain way, and if included, in order for the rule to pass, the expectation should be that the thing the rule checks for should exist and be valid/used correctly.

   Examples here are the rules that check for different fields of the manifest file. They should not pass if, for example, the web manifest file doesn't exist (even if there is a rule that checks exactly that). Furthermore, I think it's better in these types of cases for users to see all errors. That way they can understand the big picture, not that they need to fix one thing, and after they do, 5 other things pop up, and so on.

   Note: Currently we allow these types of rules to pass in cases as the ones described above, so I'll update the rules in the following days to no longer do that.

2. What the rule check is not mandatory

   Here we have rules that guard against certain things/usage. I think for these cases it's ok/expected for the rule to pass even for cases when they don't get to check anything.

   For example, if we have a rule that checks for a deprecated JavaScript API, the rule should pass in all cases except the one in which the deprecated API is used, this includes even the case where no scripts are used on the page at all.

---

Thoughts?

[molant]

I think you are right. I think the easies (and maybe best?) way to do this is just by using the traverse::end event, or maybe a new one that implies that everything is done. These rules will listen to it and then report whatever error they need to if appropriate.

[alrra]

I think the easies (and maybe best?) way to do this is just by using the traverse::end event,

Yes, we are already doing that, and I think for most cases it should be ok.

or maybe a new one that implies that everything is done.

Works for me.

[molant]

To close this issue we should add some documentation on how to achieve this. Basically listen to the scan::end event and .report() accordingly.