rrd108 / vue-mess-detector

A static code analysis tool for detecting code smells and best practice violations in Vue.js and Nuxt.js projects
MIT License
119 stars 6 forks source link

generate skeleton for new rules #139

Closed rrd108 closed 1 month ago

rrd108 commented 1 month ago

@David-Pena I was thinking about helping other developers (and ourselves) generating new rules.

Can you please try it and share your thoughts?

At the moment you can try it by node ./src/generator/createRule.js

David-Pena commented 1 month ago

Oh so inquirir is how you can prompt things to the user. I'm learning so much with this project 🔥

This feature is awesome!

What if we add more to it, for example not just creating the rule files, but what if we add by default the rule's name to rules/rules.ts and rulesCheck, rulesReport.

What do you think?

rrd108 commented 1 month ago

What if we add more to it, for example not just creating the rule files, but what if we add by default the rule's name to rules/rules.ts and rulesCheck, rulesReport.

Yes, it should be like that.

I am not satisfied with the skeleton yet.

  1. I would like to add the line number to the output also. It would be nice to have it everywhere possible, so I think the skeleton should contain it.
  2. Somehow I want to mark more clearly what to change in the generated files.
David-Pena commented 1 month ago

Yes, it should be like that.

Awesome 🚀

I would like to add the line number to the output also. It would be nice to have it everywhere possible, so I think the skeleton should contain it

The lineNumber was something I struggled with my first issue in this project so I agree with you, it should be there.

Somehow I want to mark more clearly what to change in the generated files.

What about snippets behavior? you know you create a snippet in a way so the cursor is set to a specific position or it should select by default a line or something.

I'm not sure how this ⬆️ is done but some research would do it

rrd108 commented 1 month ago

Creating snippet will be a vscode only addition.

Personally I do not like them.

What about adding comments explaining what to do at the lines where we (they) should put actual code?

David-Pena commented 1 month ago

What about adding comments explaining what to do at the lines where we (they) should put actual code?

I don't know why but I thought you discarded comments for this. If that's not the case, then yes, comments would be the most appropiate.

This is how you see it?

const checkThisIsATest = (script: SFCScriptBlock | null, filePath: string) => {
  // TODO: Check if the `script` is null
  if (!script) {
    return
  }

  // TODO: Create regex to match the pattern for this rule
  const regex = createRegExp()
  const matches = script.content.match(regex)

  // TODO: Push to `results` array if matches are found
  if (matches?.length) {
    results.push({ 
      filePath, 
      message: `add_your_message ${BG_ERR}(${matches.length})${BG_RESET}`
    })
  }
}

or you mean no code at all, just comments explaining and giving context of what to do?

rrd108 commented 1 month ago

Ah yeah I like this approach adding TODO comments. Simple, clear, easy to understand and follow.

Tomorrow I will update the skeleton. If nobody does it earlier 😉

David-Pena commented 1 month ago

Tomorrow I will update the skeleton. If nobody does it earlier 😉

haha I have one issue assigned already 👀

rrd108 commented 1 month ago

Can you check and tell me what do you think? It is in 139-generate-skeleton-for-new-rules branch

David-Pena commented 1 month ago

Hi @rrd108

I really like the structure you did. I think it achieves what we wanted with this feature to make the process to add new rules so much smoother to new contributors (and us) 🙌

I see one typo: image

And also, in both files the rule field must be `${TEXT_INFO}RULESET ~ RULENAME${TEXT_RESET}, where:

Result ⬇️

results.forEach((result) => {
      offenses.push({
        file: result.filePath,
        rule: `${TEXT_INFO}rrd ~ complicated conditions${TEXT_RESET}`,
        description: `👉 ${TEXT_WARN}/* TODO tip to fix this issue */.${TEXT_RESET} See: https:///* TODO doc link */`,
        message: `${result.message} 🚨`,
      })
})
expect(reportComplicatedConditions()).toStrictEqual([{
      file: fileName,
      rule: `${TEXT_INFO}rrd ~ complicated conditions${TEXT_RESET}`,
      description: `👉 ${TEXT_WARN}/* TODO tip to fix this issue */.${TEXT_RESET} See: https:///* TODO doc link */`,
      message: `line #/* TODO line number from your content above*/ ${BG_WARN}/* TODO message from the rule file */${BG_RESET} 🚨`,
    }])
rrd108 commented 1 month ago

Does it mean you like it in general?

rrd108 commented 1 month ago

Ah I see you answered that also.

Thanks I will fix it and merge it

David-Pena commented 1 month ago

Does it mean you like it in general?

Yes 🔥 I edited the order of my comment haha

rrd108 commented 1 month ago

implemented in https://github.com/rrd108/vue-mess-detector/releases/tag/v0.32.0