Check bugprone-unchecked-optional-access in Engine clang-tidy.

[zanderso]

There are a number of existing violations to work through (flutter/engine#42281), so we'll have to attack this in two stages:

• Enable the lint, and opt-out files that have warnings to fix - done in Enable bugprone-unchecked-optional-access, disable in files with warnings engine#42359
• Incrementally fix all the files with warnings.

[chinmaygarde]

As I said here, we could also opt in all Impeller components by modifying the impeller_component template.

[zanderso]

This check is now enabled in all non-unit test files. Unchecked access is a common pattern in unit tests, and the tests will fail on bad optional accesses, so the lint likely has limited value there. So I'll close this issue as done.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

[flar]

I'm just running into this now in the new DL geometry class tests where I have methods that return optional values. I would be happy if clang-tidy could be taught to recognize ASSERT_TRUE(result.has_value()); as protective since it returns if the test fails, but clang-tidy still warns about accesses in the subsequent code. There may be an issue in that ASSERT_TRUE is a macro that might not be expanded to return from the function in the environment under which clang-tidy is running? (But I thought it ran under host_debug which should have the macros functional?)

[flar]

Meanwhile, I've had good luck with the following macro in my unit test files. I just switch to ENFORCE_TRUE(result.has_value()) and clang-tidy stops complaining...

// ASSERT_TRUE(result.has_value()) should be enough to prevent the test
// code that follows it to run, but clang-tidy doesn't seem to recognize
// that macro expansion as protective. This macro is more proactive
// about ending execution at the place where the statement lives.
#define ENFORCE_TRUE(condition) \
  do {                          \
    if (!(condition)) {         \
      ASSERT_TRUE(condition);   \
      return;                   \
    }                           \
  } while (0)

[zanderso]

Yeah, due to this problem, we have the clang-tidy check disabled in most(all?) unit test files. If you have a good way forward, we should get that macro added to a common location, so that we can enable the check in more places.