Testing with Flow Static Analysis Tool

[karengc27]

The Flow static testing tool was added to the repository and was used to run tests on the /src folder. The output of the tests were put into nodebb-f24-team-sweepers/flow-output.txt. A screenshot of what the output of the test looks like can be seen below:

To be able to run the tests the following commands work:
npm run flow (runs tests on files with "// @ flow" at the top of file)
npx flow check --all (runs test on all files based on restrictions in .flowconfig)

The following files were added:
.flowconfig
flow-output.txt (to show outputs from tests)

The following changes were made to the package.json:

WHY WE DID NOT FIX ALL THE ERRORS:
NOTE: Despite passing the "Lint and Test" workflow, the PR for Flow integration did not clear the type checks due to the sheer volume of errors. However, we opted to merge the PR, based on the belief that many errors reported by Flow were false positives. For example, the first couple of errors given by Flow are “cannot-resolve-module” for modules such as sanitize-html. However, when one goes to the file, modules like sanitize-html were initialized in another file such as "node_modules/@types/ sanitize-html/index". A similar error given by Flow was the prop-missing error which is often caused by properties being defined in other files. For example, Flow marked “file.walk” as an error since “.walk” was not defined in the search.js file, however this property was defined in file.js. Additionally, there are also many signature-verification-failure and missing-local-annot errors that would require us to add type annotations to all the files that Flow is checking. This would require a large amount of time and effort since there are 19471 errors. After discussing this with Professor Hilton, we have decided to prioritize integrating Flow without resolving all identified errors, allowing us to continue development while gradually improving type coverage over time.

Tool Evaluation:
What types of problems are you hoping your tooling will catch? What types of problems does this particular tool catch?
I was hoping that the tool would help catch type errors since it is a static type checker for Javascript which can help with refactoring code cautiously without having to worry about causing the code to break. This in turn would help to safely merge branches into main when major code changes are made. The tool checks for various type errors and the error message often involve missing annotations, can't resolve module messages, property missing and more. These errors look like prop-missing, missing-local-annot, incompatible-use, invalid-export, cannot-resolve-module, and more. Overall, Flow tries to ensure that changes made work as intended without having any type errors occurred based on the logic.

What types of customization are possible or necessary?
For customizations, it's possible to choose which files to test. For example you can test every file by running npx flow check --all. In the .flowconfig, you can include which files or folder you want to be ignored when running"npx flow check --all" if you don't actually want to test each file. If you want to test individual file, you can run the comman "npm run flow" which would test each individual file that has "// @ flow" at the beginning of the file. You can also have flow running in the background to offer incremental feedback as you code through flow's watch mode by running the command "npx flow start". The "[options]" part of .flowconfig also allows for customization by enabling or disabling certain features such as being able to set include_warnings to true. Other parts of the .flowconfig file that allow for customization are [untyped], [declarations], [libs], [lints], and [strict].

How can/should this tool be integrated into a development process?
To integrate this tool, you have to run npm install --save-dev flow-remove-types to install flow-remove-types. This has to be added to scripts in the package.json: "build": "flow-remove-types src/ -d lib/", "prepublish": "npm run build". Then flow bin has to be installed: npm install --save-dev flow-bin. This has to be added to scripts in package.json: "flow": "flow". The first time you run flow this command has to be used: npm run flow init. After to run flow this command can be used: npm run flow. Changes can be made to .flowconfig to choose which files should be tested or ignored.

Are there many false positives? False negatives? True positive reports about things you don’t care about?
I think that there are not many false positives or negatives, however it points out a lot of true positive reports about things we don't care about. For example, when running the tests there were a lot of errors about missing annotations, however the code still runs as intended and the error acts as more of a warning. It is true that the errors that it gives could pose a potential issue in the future. For example, for nodebb-f24-team-sweepers/src/topics/tools.js it gave 50 errors which most were "missing-local-annot"

[coveralls]

Pull Request Test Coverage Report for Build 11580216013

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 82.69%

---

Totals
Change from base Build 11467018223:	0.0%
Covered Lines:	22331
Relevant Lines:	25587

---

💛 - Coveralls

[Alanna-Cao]

Looks good to me!