Skip to content

Commit 233fe76

Browse files
committed
xtask: Copyright and license lints for more file types
Few files turned out to not have the notes, and they were fixed manually. Provide a robust solution by updating house rules in xtask. Additionally this includes an updatex for the automatic fix mode. I made sure that produces desired results and is idempotent Suggested-by: Daniel Prilik <[email protected]> Reported-by: Matt Kurjanowicz <[email protected]> Fixes: #766
1 parent deb2792 commit 233fe76

File tree

5 files changed

+77
-19
lines changed

5 files changed

+77
-19
lines changed

.github/scripts/add_unsafe_reviewers/add-unsafe-reviewers.py

+3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# Licensed under the MIT License.
3+
14
import click
25
from git import Repo
36
from github import Github

.github/scripts/refresh_mirror/refresh-mirror.py

+3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# Licensed under the MIT License.
3+
14
import click
25
import time
36
import sys

support/clap_dyn_complete/src/templates/complete.ps1

+3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# Licensed under the MIT License.
3+
14
Register-ArgumentCompleter -Native -CommandName '__COMMAND_NAME__' -ScriptBlock {
25
param($wordToComplete, $commandAst, $cursorPosition)
36

vmm_tests/vmm_tests/test_data/tpm.py

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# Copyright (c) Microsoft Corporation.
22
# Licensed under the MIT License.
3+
34
#
45
# The test script that reads AK CERT from TPM and validate its content
56

xtask/src/tasks/fmt/house_rules/copyright.rs

+67-19
Original file line numberDiff line numberDiff line change
@@ -17,24 +17,46 @@ pub fn check_copyright(path: &Path, fix: bool) -> anyhow::Result<()> {
1717
.and_then(|e| e.to_str())
1818
.unwrap_or_default();
1919

20-
if !matches!(ext, "rs" | "c" | "proto" | "toml" | "ts" | "js") {
20+
if !matches!(
21+
ext,
22+
"rs" | "c" | "proto" | "toml" | "ts" | "js" | "py" | "ps1" | "config"
23+
) {
2124
return Ok(());
2225
}
2326

2427
let f = BufReader::new(File::open(path)?);
2528
let mut lines = f.lines();
26-
let first_line = lines.next().unwrap_or(Ok(String::new()))?;
27-
let second_line = lines.next().unwrap_or(Ok(String::new()))?;
28-
let third_line = lines.next().unwrap_or(Ok(String::new()))?;
29+
let (script_interpreter_line, blank_after_script_interpreter_line, first_content_line) = {
30+
let line = lines.next().unwrap_or(Ok(String::new()))?;
31+
// Besides the "py", "ps1, "toml", and "config" files, only for Rust,
32+
// `#!` is in the first set of the grammar. That's why we need to check
33+
// the extension for not being "rs".
34+
// Someone may decide to put a script interpreter line (aka "shebang")
35+
// in a .config or a .toml file, and mark the file as executable. While
36+
// that's not common, we choose not to constrain creativity.
37+
if line.starts_with("#!") && ext != "rs" {
38+
let script_interpreter_line = line;
39+
let after_script_interpreter_line = lines.next().unwrap_or(Ok(String::new()))?;
40+
(
41+
Some(script_interpreter_line),
42+
Some(after_script_interpreter_line.is_empty()),
43+
lines.next().unwrap_or(Ok(String::new()))?,
44+
)
45+
} else {
46+
(None, None, line)
47+
}
48+
};
49+
let second_content_line = lines.next().unwrap_or(Ok(String::new()))?;
50+
let third_content_line = lines.next().unwrap_or(Ok(String::new()))?;
2951

3052
// Preserve any files which are copyright, but not by Microsoft.
31-
if first_line.contains("Copyright") && !first_line.contains("Microsoft") {
53+
if first_content_line.contains("Copyright") && !first_content_line.contains("Microsoft") {
3254
return Ok(());
3355
}
3456

35-
let mut missing_banner =
36-
!first_line.contains(HEADER_MIT_FIRST) || !second_line.contains(HEADER_MIT_SECOND);
37-
let mut missing_blank_line = !third_line.is_empty();
57+
let mut missing_banner = !first_content_line.contains(HEADER_MIT_FIRST)
58+
|| !second_content_line.contains(HEADER_MIT_SECOND);
59+
let mut missing_blank_line = !third_content_line.is_empty();
3860

3961
// TEMP: until we have more robust infrastructure for distinct
4062
// microsoft-internal checks, include this "escape hatch" for preserving
@@ -44,8 +66,9 @@ pub fn check_copyright(path: &Path, fix: bool) -> anyhow::Result<()> {
4466
let is_msft_internal = std::env::var("XTASK_FMT_COPYRIGHT_ALLOW_MISSING_MIT").is_ok();
4567
if is_msft_internal {
4668
// support both new and existing copyright banner styles
47-
missing_banner = !(first_line.contains("Copyright") && first_line.contains("Microsoft"));
48-
missing_blank_line = !second_line.is_empty();
69+
missing_banner =
70+
!(first_content_line.contains("Copyright") && first_content_line.contains("Microsoft"));
71+
missing_blank_line = !second_content_line.is_empty();
4972
}
5073

5174
if fix {
@@ -64,10 +87,19 @@ pub fn check_copyright(path: &Path, fix: bool) -> anyhow::Result<()> {
6487
let mut f = BufReader::new(File::open(path)?);
6588
let mut f_fixed = File::create(path_fix)?;
6689

90+
if let Some(script_interpreter_line) = script_interpreter_line {
91+
writeln!(f_fixed, "{script_interpreter_line}")?;
92+
}
93+
if let Some(blank_after_script_interpreter_line) = blank_after_script_interpreter_line {
94+
if !blank_after_script_interpreter_line {
95+
writeln!(f_fixed)?;
96+
}
97+
}
98+
6799
if missing_banner {
68100
let prefix = match ext {
69101
"rs" | "c" | "proto" | "ts" | "js" => "//",
70-
"toml" => "#",
102+
"toml" | "py" | "ps1" | "config" => "#",
71103
_ => unreachable!(),
72104
};
73105

@@ -101,17 +133,33 @@ pub fn check_copyright(path: &Path, fix: bool) -> anyhow::Result<()> {
101133
}
102134
}
103135

104-
let msg = match (missing_banner, missing_blank_line) {
105-
(true, true) => "missing copyright & license header + subsequent blank line",
106-
(true, false) => "missing copyright & license header",
107-
(false, true) => "missing blank line after copyright & license header",
108-
(false, false) => return Ok(()),
109-
};
136+
// Consider using an enum if there more than three,
137+
// or the errors need to be compared.
138+
let mut missing = vec![];
139+
if missing_banner {
140+
missing.push("the copyright & license header");
141+
}
142+
if missing_blank_line {
143+
missing.push("a blank line after the copyright & license header");
144+
}
145+
if let Some(blank_after_script_interpreter_line) = blank_after_script_interpreter_line {
146+
if !blank_after_script_interpreter_line {
147+
missing.push("a blank line after the script interpreter line");
148+
}
149+
}
150+
151+
if missing.is_empty() {
152+
return Ok(());
153+
}
110154

111155
if fix {
112-
log::info!("fixed {} in {}", msg, path.display());
156+
log::info!(
157+
"applied fixes for missing {:?} in {}",
158+
missing,
159+
path.display()
160+
);
113161
Ok(())
114162
} else {
115-
Err(anyhow!("{} in {}", msg, path.display()))
163+
Err(anyhow!("missing {:?} in {}", missing, path.display()))
116164
}
117165
}

0 commit comments

Comments
 (0)