Script to add new rule file

[acharles7]

Summary

Fixes #119

• Added script to create new_rule.py

$ python -m fixit.cli.add_new_rule --path fixit/rules/new_rule.py

This will create a skeleton of the rule file at the path given in the --path argument

new_rule.py

# Copyright (c) Facebook, Inc. and its affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.


from fixit import CstLintRule, InvalidTestCase as Invalid, ValidTestCase as Valid


"""
This is a model rule file for adding a new rule to fixit module
"""


class Rule(CstLintRule):
    """
    docstring or new_rule description
    """

    MESSAGE = "Enter rule description message"

    VALID = [Valid("'example'")]

    INVALID = [Invalid("'example'")]

[jimmylai]

Let's also include this tool in the documentation.

https://github.com/Instagram/Fixit/blob/46a0d0b66e494c7dad4fe5d57dea9981a96df322/docs/source/build_a_lint_rule.ipynb

You can run jupyter notebook in your local venv to start Jupyter notebook and edit it in browser.
It uses nbsphinx to generate doc and text cell needs to use rst format.
https://nbsphinx.readthedocs.io/en/0.7.1/raw-cells.html
The best place to mention this command is in the section of "Lint Rule Scaffolding".

[isidentical]

As far as I could estimate from writing 2 rules is that, saving the rule to new_file.py by default isn't that useful. Maybe it can take a positional argument named rule name and use it on both places (the filename and the class name, so that people wouldn't need to replace the class name, because they might just remove the Rule at all which is non-compliant with the style standards of this repo) (just my opinion)

[acharles7]

As far as I could estimate from writing 2 rules is that, saving the rule to new_file.py by default isn't that useful. Maybe it can take a positional argument named rule name and use it on both places (the filename and the class name, so that people wouldn't need to replace the class name, because they might just remove the Rule at all which is non-compliant with the style standards of this repo) (just my opinion)

I already suggest this, but As per @jimmylai suggestion, I have not implemented this functionality.

[acharles7]

As far as I could estimate from writing 2 rules is that, saving the rule to new_file.py by default isn't that useful. Maybe it can take a positional argument named rule name and use it on both places (the filename and the class name, so that people wouldn't need to replace the class name, because they might just remove the Rule at all which is non-compliant with the style standards of this repo) (just my opinion)

I already suggest this, but As per @jimmylai suggestion, I have not implemented this functionality.

Will add --name abcd later on for

• create filename, abcd.py
• class name, class AbcdRule

[jimmylai]

As far as I could estimate from writing 2 rules is that, saving the rule to new_file.py by default isn't that useful. Maybe it can take a positional argument named rule name and use it on both places (the filename and the class name, so that people wouldn't need to replace the class name, because they might just remove the Rule at all which is non-compliant with the style standards of this repo) (just my opinion)

I already suggest this, but As per @jimmylai suggestion, I have not implemented this functionality.

The class name is in camel case and the file name is in underscore-separated-case. They doesn't necessary to be the same name. So provide another parameter like --rule or --name for class name makes more sense to me.

[isidentical]

The class name is in camel case and the file name is in underscore-separated-case. They doesn't necessary to be the same name. So provide another parameter like --rule or --name for class name makes more sense to me.

Yeah, makes sense (the thing that was in my mind was .split('_') + .title() + .join() + 'Rule')

[acharles7]

The class name is in camel case and the file name is in underscore-separated-case. They doesn't necessary to be the same name. So provide another parameter like --rule or --name for class name makes more sense to me.

Yeah, makes sense (the thing that was in my mind was .split('_') + .title() + .join() + 'Rule')

How about this?

def snake_to_camelcase(name):
  """Convert snake-case string to camel-case string."""
  return "".join(n.capitalize() for n in name.split("_"))

[jimmylai]

The class name is in camel case and the file name is in underscore-separated-case. They doesn't necessary to be the same name. So provide another parameter like --rule or --name for class name makes more sense to me.

Yeah, makes sense (the thing that was in my mind was .split('_') + .title() + .join() + 'Rule')

How about this?

def snake_to_camelcase(name):
  """Convert snake-case string to camel-case string."""
  return "".join(n.capitalize() for n in name.split("_"))

Yeah, that works. Then we should explain it clearly in the documentation.

[acharles7]

@jimmylai Should I start working on --name or Is it for the future?

[jimmylai]

@jimmylai Should I start working on --name or Is it for the future?

We can leave the name parameter change as the next PR and keep this PR simpler.

[jimmylai]

It's getting closer.
The added commands doesn't work and show this error.

It's because the working directory is docs/source, rather than the root of project, so it cannot find fixit/rules.

Let's change the working directory with a hidden cell.

To hide the os.chdir cell, you can click View to enable the Edit Metadata button and then add "nbsphinx": "hidden" to the metadata to hide it. So it won't be included in the generated html.

Previously, the add_rule command is in the same cell as the cat which their output will print together.
Let's also separate them as two different cell to be clear.
Lastly, the documentation you added is better for "build_a_lint_rule". Add it to "getting_started" looks redundant. So let's not to add it to "getting_started".

[jimmylai]

@acharles7 it looks really awesome now!
Thanks for your contribution and sorry for so many back and forth.

[acharles7]

@jimmylai Thank you for your quick reviews.
Should I start working on --name?

[jimmylai]

@jimmylai Thank you for your quick reviews.
Should I start working on --name?

Sure, please go ahead.