-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Remove the term "whitelist" #3507
Remove the term "whitelist" #3507
Conversation
We have a couple of places where we check that the set of analyzer assemblies is internally consistent and complete--that is, for any given assembly in that set, all of its dependencies are satisfied by other assemblies in the set. We expect certain dependencies to be satisfied by the host, and so want to ignore them if they are not found in the the analyzer assembly set. We referred to these host-provided dependencies as a white list, but Policheck does not like that term. Instead, we now refer to these as the "ignorable assembly list".
👍 |
Approved on 6/15 by ML Shiproom. This was already approved in a PR against master #3457 |
Remove the term "whitelist" **Bug:** Fixes DevDiv Bug 1185799 **Customer Scenario** No customer scenario, per se. There are a number of places where we use the term "whitelist", and one of them (a code comment) was flagged by Policheck. Given that our code is open I've gone ahead and simply fixed them all. **Fix** Several internal types were updated with new names, as well as various fields, locals, and a comment. **Testing** After the fix, the code still passes all of our unit and integration tests.
I found the word |
I can't even...why is this a thing? |
This is wrong on so many levels. We are programmers, not politicians ffs! |
at the risk of stirring up a hornets nest, ignorelist is quite possibly one of the worst names for whitelist/blacklist. Does ignore mean allow, or does ignore mean deny. In this case it seems like you've used ignorelist to mean whitelist, which totally doesn't make sense to me. I'd propose |
Totally agree with @crowell. IMO, |
Agree with @crowell and @indualagarsamy. Clarity ftw. |
Seriously? It smells like racism bullsh*t for me. Black and whitelist words have a way different etymology. In my language translating whitelist or black list to block or accept list will be nice and long joke. |
Blocklist!? No way. Now that is offensive. My dad used to be a factory worker making blocks. You making fun of blocks now too with your "programming" skills? Actually it seems discriminating to "allow", "block", or "ignore" anything. This shit needs to be completely removed from modern day software. Things like firewalls make my blood boil. Discriminating network packets and the free flow of digital data. You programmers. It's inexcusable how you're getting away with this behavior. |
Instead of fixing the compiler, you should just whitelist |
Why you're so racist and removing things with word "white"? :> |
I totally agree. Additionally, I think we should remove static typing. We shouldn't be telling objects what they are until they decide for themselves, at runtime. Segregating objects based on type? I think I've heard that before... |
I am feeling offended by anyone offended by word such as "blacklist", "master-slave". Please check your privilege and reconsider your priorities. |
great job, now it's time for https://github.com/torvalds/linux/search?p=1&q=blacklist&utf8=%E2%9C%93 please do, I want to see that. |
👍 by me. This backlash by angry brogrammers was to be expected. It's like you just wait all day to find something to throw a temper tantrum about. Your lingo might have been ok back in the day, but nowadays we actually care about being mindful about each others backgrounds. If someone tells you that the words you're using are not ok, just be nice and don't use them, it's that simple. Our language changes, all the time, deal with it. |
@quizix You're the ones finding something to be angry about. There is no need to change terminology here. |
Folks... this is a 1 year old PR. Let's just leave it. It's silly, but that's the way it is. |
Comply, comply! @quizlx This isn't a change of words that are widely regarded as offensive. This is a blanket ban on words that have no relation to anything remotely offensive to any sane person. Etymology does not hint any racial load neither: https://en.wikipedia.org/wiki/Blacklisting so why is language gutted? |
@quizlx @lbialy
You make exactly the right point. Words like master-slave, whitelisting, blacklisting have been used by programmers for decades. Way before most people here were even born. They were never even remotely intended to be used for anything else than conveying technical information, and getting the job done. No sinister thought or offensive idea even crossed the minds of the people using these terms. They are de-facto standard words used to describe technical computing terms, nothing else. Now here then comes a group from the so called generation snowflake, and they MAKE UP meaning where none was before, and then after they establish what they think is the new meaning they start to demand change. It is as if I would suddenly start to accuse Github for designing this website with black text on white background. Is Github insinuating something with this color scheme? Black on white? Imagine the shitstorm we could raise demanding that black text on white background has to be banned, because I came up with this offensive meaning for this type of color scheme. This whole PC trend in programming is an entirely made up fairy tale by some people who cannot find any other way to contribute constructively to society. It's easier to control people by telling them things that need to be done and changed, than to do something constructive yourself. Make up your own fantasy, and impose it on everyone else. |
|
In ancient China, there was a crime called “Literary inquisition”. It was designed by government to make people to shut up.After about 300 years, Chinese citizens had revoluted for so many times.But we haven't get rid of it.Today I found the Literary inquisition exist in free countries,too. |
left-handed wanna say: STOP using Right/LEFT to stand for what is good/the rest |
可以,这很清真 |
It doesn't need to.
And in the past, it was completly acceptable for people to use the label "n*gro". It had a different meaning back then, even MLK used it. Would you argue that for this reason, it is not offensive today? Go ahead then, use it. Use it and see how people react.
Except duh Microsoft changed the wording out of their own volition. |
@quizlx I am from China.So my English is poor. I think you may misunderstand me. The "Literary inquisition" is a crime which was designed by gov. People whose words touch the gov's weakness will be throwed into jail. The "Literary inquisition" often happen in some tiny places, like homophonic, spell & double meaning. Nobody like Racial Discrimination. Asian sometimes are victims, too. But the white-list is just a nickname. People who designed such words may be not intentional. In China, there is a kind of website called yellow page. No body think they are making fun of yellow skin Asian. Even in China, yellow represents porn. |
next: remove the master-branch. It implies that there could be a slave-branch. So having a master-branch is a potential sign for supporting slavery. Seriously, wake up! All this sjw-code-of-conduct-stuff is totally nonsense. Pls spend you time on fixing real bugs instead. |
Note how cultural marxists never want to create their own software and communities.Instead they want to police and control software created by others, under the guise of "inclusivity". |
I have a counterproposal, for the sake of clarity and better representation for ethnic minorities. Rename whitelist to 'supremepalegoodgoys' and blacklist to 'untermenchafrosemitics'. Will any of the people with commit bits back this proposal ? Hail Hydra |
Perhaps TBH I would have left it as blacklist/whitelist because it's commonly used and understood, and clarity trumps everything when writing shared code. |
@pHr34kY why are you even taking this seriously? |
Why did this got approved, is a non sense issue... we are programmers not policial class people so we do not need to be sensible to some but hurt people for a word... is a word!!! |
This change makes no sense simply because of the clarity you're trying to preserve. The commonly accepted term for a list of people who can't use something is called a blacklist. Changing the term destroys the lingo that has been developed, and now people looking at your code from an outsiders view who haven't seen this pull. |
ITT: people who never touched this project suddenly have some strong words to say in yet another pointless "I AM OFFENDED BY THIS" drama drama people pls go |
@moopie 👌👌👌👌👌 |
@@ -12,19 +12,19 @@ namespace Microsoft.CodeAnalysis.CompilerServer | |||
{ | |||
internal static class AnalyzerConsistencyChecker | |||
{ | |||
private static readonly ImmutableArray<string> s_defaultWhiteList = ImmutableArray.Create("mscorlib", "System", "Microsoft.CodeAnalysis"); | |||
private static readonly ImmutableArray<string> s_defaultIgnorableReferenceNames = ImmutableArray.Create("mscorlib", "System", "Microsoft.CodeAnalysis"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change to s_defaultSafeReferenceNames
|
||
public static bool Check(string baseDirectory, IEnumerable<CommandLineAnalyzerReference> analyzerReferences, IAnalyzerAssemblyLoader loader, IEnumerable<string> referenceWhiteList = null) | ||
public static bool Check(string baseDirectory, IEnumerable<CommandLineAnalyzerReference> analyzerReferences, IAnalyzerAssemblyLoader loader, IEnumerable<string> ignorableReferenceNames = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change to safeReferenceNames
@moopie: cause software development is not about political corectness. This is not right or wrong. This is just strange what's happened in this commit! And yeah. We can say we are offended by this, like author can say he's offended by words "Whitelist" and "Blacklist" that were used in IT for years. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should really rename to the meaning intended (not ignore but skip or accept or reject)
{ | ||
if (referenceWhiteList == null) | ||
if (ignorableReferenceNames == null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change to safeReferenceNames
{ | ||
referenceWhiteList = s_defaultWhiteList; | ||
ignorableReferenceNames = s_defaultIgnorableReferenceNames; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change to safeReferenceNames
} | ||
|
||
try | ||
{ | ||
CompilerServerLogger.Log("Begin Analyzer Consistency Check"); | ||
return CheckCore(baseDirectory, analyzerReferences, loader, referenceWhiteList); | ||
return CheckCore(baseDirectory, analyzerReferences, loader, ignorableReferenceNames); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change to safeReferenceNames
@@ -37,7 +37,7 @@ public static bool Check(string baseDirectory, IEnumerable<CommandLineAnalyzerRe | |||
} | |||
} | |||
|
|||
private static bool CheckCore(string baseDirectory, IEnumerable<CommandLineAnalyzerReference> analyzerReferences, IAnalyzerAssemblyLoader loader, IEnumerable<string> referenceWhiteList) | |||
private static bool CheckCore(string baseDirectory, IEnumerable<CommandLineAnalyzerReference> analyzerReferences, IAnalyzerAssemblyLoader loader, IEnumerable<string> ignorableReferenceNames) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change to safeReferenceNames
foreach (var resolvedPath in resolvedPaths) | ||
{ | ||
var missingDependencies = AssemblyUtilities.IdentifyMissingDependencies(resolvedPath, resolvedPaths); | ||
|
||
foreach (var missingDependency in missingDependencies) | ||
{ | ||
if (!referenceWhiteList.Any(name => missingDependency.Name.StartsWith(name))) | ||
if (!ignorableReferenceNames.Any(name => missingDependency.Name.StartsWith(name))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change to safeReferenceNames
private readonly IBindingRedirectionService _bindingRedirectionService; | ||
|
||
public AnalyzerDependencyChecker(IEnumerable<string> analyzerFilePaths, IEnumerable<IAssemblyWhiteList> assemblyWhiteLists, IBindingRedirectionService bindingRedirectionService = null) | ||
public AnalyzerDependencyChecker(IEnumerable<string> analyzerFilePaths, IEnumerable<IIgnorableAssemblyList> ignorableAssemblyLists, IBindingRedirectionService bindingRedirectionService = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change to IEnumerable<ISkippableAssemblyList> skippableAssemblyList
{ | ||
Debug.Assert(analyzerFilePaths != null); | ||
Debug.Assert(assemblyWhiteLists != null); | ||
Debug.Assert(ignorableAssemblyLists != null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change to skippableAssemblyList
|
||
_analyzerFilePaths = new HashSet<string>(analyzerFilePaths, StringComparer.OrdinalIgnoreCase); | ||
_assemblyWhiteLists = assemblyWhiteLists.ToList(); | ||
_ignorableAssemblyLists = ignorableAssemblyLists.ToList(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change to _skippableAssemblyList = skippableAssemblyList.ToList()
@@ -46,7 +46,7 @@ public AnalyzerDependencyResults Run(CancellationToken cancellationToken = defau | |||
} | |||
} | |||
|
|||
_assemblyWhiteLists.Add(new AssemblyIdentityWhiteList(analyzerInfos.Select(info => info.Identity))); | |||
_ignorableAssemblyLists.Add(new IgnorableAssemblyIdentityList(analyzerInfos.Select(info => info.Identity))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change to _skippableAssemblyList.Add(new SkippableAssemblyIdentityList(analyzerInfos.Select(info => info.Identity)));
@@ -74,7 +74,7 @@ private ImmutableArray<MissingAnalyzerDependency> FindMissingDependencies(List<A | |||
? _bindingRedirectionService.ApplyBindingRedirects(reference) | |||
: reference; | |||
|
|||
if (!_assemblyWhiteLists.Any(whiteList => whiteList.Includes(redirectedReference))) | |||
if (!_ignorableAssemblyLists.Any(ignorableAssemblyList => ignorableAssemblyList.Includes(redirectedReference))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change to if (!_skippableAssemblyLists.Any(skippableAssemblyList => skippableAssemblyList.Includes(redirectedReference)))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is stupid, everyone knows what whitelist means, and ignorableAssemblyList means nothing on the first read.
This PR is linked on /r/ProgrammerHumor hence the comments. PR/merge worthy project here: https://github.com/ErisBlastar/cplusequality |
What about compiling to gender neutral non-binary bits instead? |
was thinking quacklist and honklist would be suitable |
Can we replace all Booleans with floats as well? The reasoning is two-fold:
The obvious solution is to replace bools with floats. (The above is satire, just to be perfectly clear.) |
@StevenGann PR underway! |
This is interesting, where can I find a copy of Policheck? |
@tekguy don't click on that above link! Completely NSFL! If you do an internet search you can find a presentation on PoliCheck on prezi.com. |
This comment has been minimized.
This comment has been minimized.
@tekguy: just don't search for that at all. Everyone will have better sleep. |
Thanks! |
Note that this fix has been to ship room before and was approved (request #3457); I then discovered I had created the pull request against master instead of stabilization. This is the same change, against stabilization.
Bug: Fixes DevDiv Bug 1185799
Customer Scenario
No customer scenario, per se.
There are a number of places where we use the term "whitelist", and one of them (a code comment) was flagged by Policheck. Given that our code is open I've gone ahead and simply fixed them all.
Fix
Several internal types were updated with new names, as well as various fields, locals, and a comment.
Testing
After the fix, the code still passes all of our unit and integration tests.
@ManishJayaswal This is ready to go to ship room.