Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: refactor rules to put each rule into directory of their category #4301

Closed
wants to merge 2 commits into from

Conversation

straker
Copy link
Contributor

@straker straker commented Jan 16, 2024

Just bringing some organization to the lib/rules directory that we've talked about before. Each rule is now put into a directory for the rules category, similar to how each check is in a directory for the category. Matches files are placed as a sibling to the rule that uses it, unless multiple rules from different categories make use of it in which case it goes into the generic dir (similar to the checks generic dir).

Used the following script to do 99% of the work, and manually fixed a a few instances of matches depending on another matches file.

const { promises: fs } = require('fs');
const path = require('path');
const { glob } = require('glob');
const util = require('util');
const exec = util.promisify(require('child_process').exec);

const matchesFiles = {};

(async () => {
  const rulesDir = './lib/rules';
  const files = await glob(path.join(rulesDir, '*.json'), { dotRelative: true });
  for (const file of files) {
    const rule = require(file);

    const matches = rule.matches;
    const cat = rule.tags.find(tag => tag.startsWith('cat.'));
    const catDir = cat.split('.')[1];

    const filename = path.basename(file);

    try {
      await exec(`mkdir ${path.join(rulesDir, catDir)}`);
    } catch (err) {}
    await exec(`git mv ${path.join(rulesDir, filename)} ${path.join(rulesDir, catDir, filename)}`);

    if (matches) {
      matchesFiles[matches + '.js'] = matchesFiles[matches + '.js'] ?? [];
      matchesFiles[matches + '.js'].push(catDir);
    }
  }

  for (matchFile in matchesFiles) {
    const matches = matchesFiles[matchFile];
    let catDir;

    if (matches.every(catDir => catDir === matches[0])) {
      catDir = matches[0];
    }
    else {
      catDir = 'generic';
      try {
        await exec(`mkdir ${path.join(rulesDir, catDir)}`);
      } catch (err) {}
    }

    const filePath = path.join(rulesDir, matchFile);
    let file = await fs.readFile(filePath, 'utf8');
    file = file.replaceAll(/ '\.\.\//g, " '../../");
    await fs.writeFile(filePath, file, 'utf8');

    await exec(`git mv ${path.join(rulesDir, matchFile)} ${path.join(rulesDir, catDir, matchFile)}`);
  }
})();

@straker straker requested a review from a team as a code owner January 16, 2024 20:49
Copy link
Contributor

@dbjorge dbjorge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for including the script you used to generate the change, it was helpful for reviewing!

The changes that are here look fine, but I think there are a few minor omissions:

  • There are a few orphaned files left in /lib/rules that appear to be dead code; they can probably be removed
  • There are a few links to files within the rules directory in some of the developer documentation that should be updated (search for /rules/, there are cases in /doc/check-options.md and /doc/developer-guide.md)

@straker
Copy link
Contributor Author

straker commented Jun 18, 2024

Closing as we decided not to do this.

@straker straker closed this Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants