pylint checker for copyright notices

[vtomole]

Description of the issue
Maybe pylint could automatically check for copyright notices instead of reviewers needed to keep an eye out. Inspired by #456.

Look into https://stackoverflow.com/questions/51886592/can-pylint-check-for-a-static-comment-copyright-notice-at-the-top-of-all-docum

[yjt98765]

A 'raw checker' should be able to check it. Can you assign the task to me?

[vtomole]

@yjt98765 Assigned.

[ronnie-llamado]

Here's a tool I've seen used in other projects for a similar purpose: https://github.com/fsfe/reuse-tool

[vtomole]

Thanks for the recommendation @ronnie-llamado . We prefer to leverage tools we already have like pylint for these cases because Cirq contains too many dependencies as it is.

[yjt98765]

@yjt98765 Assigned.

Thank you!

As shown in this example, we need to assign an id (a letter + 4 digits) to the message. Since no custom checkers have been used in this project, we should come up with a rule for this, especially for the numbers (e.g., they are sequential numbers with prefix 00 or 99). The letter is one of C, W, E, F, and R, standing for Convention, Warning, Error, Fatal, and Refactoring. I guess the proper one for the copyright notice would be E as it has to be there. What do you think?

[yjt98765]

Thanks for the recommendation @ronnie-llamado . We prefer to leverage tools we already have like pylint for these cases because Cirq contains too many dependencies as it is.

It seems that the tool can be applied to many types of files, while pylint can only check python files. In this project, Some (if not all) of the bash scripts, TypeScript files, and Jupyter Notebooks also contain a copyright notice.

However, if only one type of copyright is adopted here, applying that tool seems like overkill. It complicates the checking process and increases external dependency.

[yjt98765]

@vtomole I have developed the checker and tested it locally. Before creating a pull request, I have several questions:

1. There are several versions of copyright notices existing in the project. First, the year ranges from 2018 to 2021 in the first line. Second, there are several variations regarding the 7th line:

• #http://www.apache.org/licenses/LICENSE-2.0
• # http://www.apache.org/licenses/LICENSE-2.0
• # https://www.apache.org/licenses/LICENSE-2.0 (note the difference between http and https)
  I guess all of them are fine. Therefore, I implemented the checking with Regex matching instead of String comparison. Should we allow the copyright notices to be slightly different, or do you prefer a sticker rule?

2. I have run check/pylint, and more than 300 files are reported as not having the correct copyright notice. Nearly half of them are testing files and 17 are __init__.py. Should we skip these two types of files or require every python file to have the copyright notice? I recommend checking all the files because the logic is clear, and it is easy to maintain the workflow. If some particular file does not need a copyright notice, it can disable the message in the file using # pylint: disable=wrong-copyright-notice.

[vtomole]

Therefore, I implemented the checking with Regex matching instead of String comparison. Should we allow the copyright notices to be slightly different, or do you prefer a sticker rule?

Let's change all the http links to # https://www.apache.org/licenses/LICENSE-2.0 but keep the copyright years ranging as it that specifies when the file was created and add copyright notices to all the files.

[vtomole]

guess the proper one for the copyright notice would be E as it has to be there. What do you think?

https://softwareengineering.stackexchange.com/a/19653/322605 says that it's not legally required that every file have a copyright notice (note: the poster is not a lawyer.) Keeping this in mind, I think the more appropriate emitted error would be [R]efactor for a "good practice" metric violation.

[MichaelBroughton]

This functionality is in now.